Skip to content

Commit b5c3c4c

Browse files
authored
refactor!: update Pixel subtraits (#77)
Supersedes #72 which has become a bit of a mess. I went to the std documentation to collect the traits implemented on u8/u16: - arithmetic: !, +, -, *, /, % - bitwise: |, &, ^, <<, >> - comparison: PartialEq, Eq, PartialOrd, Ord - formatting: {} {:?} {:o} {:x} {:X} {:e} {:E} {:b} - conversions: various (from/into, tryfrom/tryinto, FromStr) - common misc: Clone, Default, Copy, Hash, Sum, Step, Product - marker (nightly): ZeroablePrimitive, SimdElement, RangePattern, AtomicPrimitive, - nightly/wip misc: NumBufferTrait, FunnelShift, Distribution - misc internals: DisjointBitOr, CarrylessMul, CarryingMulAdd It's notable that a lot of operations on integers (`checked_*`, `wrapping_*`, `unsigned_abs`, `abs_diff`, `log`, `pow` etc. etc. etc.) don't live in a trait but are just type-inherent methods. Even `num_traits` doesn't expose most of these, maybe because `std` has steadily been growing over the years. I see two main problems with keeping arithmetics/bitwise operations "in" Pixel: - maintenance burden, i.e. keeping up with std/num_traits adding new stuff - questionable usability of arithmetics given type size limitations (how often do you actually want to work in a `u8`? any sum will quickly overflow) So I suggest **dropping arithmetic and bitwise operations** from the Pixel trait entirely. This requires users to convert to some concrete numeric type before "working" on pixels. Because there is exactly one of each `TryFrom`, `TryInto`, `From` and `Into` required by `T: Pixel`, types can be fully inferred even without outside "help": ```rs let a = pix.into(); // always u16 let b = pix.try_into().unwrap(); // always u8 let pix = T::from(x); // always u8 let pix = T::try_from(y).unwrap(); // always u16 ``` If you want to widen a Pixel into an integer, you can do `u64::from(pix.into())` (which is infallible so no panics ever). If you want to narrow an integer into `Pixel`, you have to do it twice though: `Pixel::try_from(my_u64.try_into().expect("fits into u16")).expect("Pixel is u16")`.
1 parent 88c3e13 commit b5c3c4c

3 files changed

Lines changed: 119 additions & 20 deletions

File tree

Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@ edition = "2024"
99
repository = "https://github.com/rust-av/v_frame"
1010
include = ["Cargo.toml", "README.md", "LICENSE", "src"]
1111

12-
[dependencies]
13-
num-traits = "0.2.19"
14-
1512
[features]
1613
padding_api = [] # exposes low-level APIs that allow bypassing padding
1714

src/pixel.rs

Lines changed: 116 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,91 @@
2121
//! The type used must match the bit depth specified when creating frames:
2222
//! - 8-bit frames must use `u8`
2323
//! - 9-16 bit frames must use `u16`
24+
//!
25+
//! # Working with `T: Pixel`
26+
//!
27+
//! It is often necessary to convert between an abstract pixel and their concrete
28+
//! integer representation in order to work with them, for example to implement an
29+
//! algorithm operating on video frames.
30+
//!
31+
//! For this purpose, `Pixel` is a supertrait of some conversions from the standard
32+
//! library, namely:
33+
//! - `Into<u16>` to convert any pixel into a `u16`,
34+
//! - `TryInto<u8>` to try to convert any pixel into a `u8` (not possible if the pixel value
35+
//! is outside of the range representable by `u8`),
36+
//! - `From<u8>` to convert any `u8` into a Pixel containing the given value and
37+
//! - `TryFrom<u16>` to try to convert any `u16` into a `T` (not possible if the source value
38+
//! is outside of the range representable by `T`).
39+
//!
40+
//! For example, to sum all pixels in a row:
41+
//!
42+
//! ```
43+
//! use v_frame::frame::Frame;
44+
//! use v_frame::pixel::Pixel;
45+
//!
46+
//! pub fn summed_rows<T: Pixel>(frame: &Frame<T>) -> Vec<u64> {
47+
//! frame.y_plane
48+
//! .rows()
49+
//! // row is &[T] here
50+
//! .map(|row| {
51+
//! row.iter()
52+
//! .map(|&pix| pix.into()) // convert T -> u16
53+
//! .map(|pix| u64::from(pix)) // widen so sum doesn't overflow
54+
//! .sum()
55+
//! })
56+
//! .collect()
57+
//! }
58+
//! ```
59+
//!
60+
//! Note that Pixel only provides one `Into<T>` implementation so it is not necessary to
61+
//! explicitly specify the wanted type (`<T as Into<u16>>::into(pix)`).
62+
//!
63+
//! Here's a somewhat contrived example of how to convert integers into pixels:
64+
//!
65+
//! ```
66+
//! use v_frame::frame::Frame;
67+
//! use v_frame::pixel::Pixel;
68+
//!
69+
//! fn random_value() -> i32 {
70+
//! // chosen by a fair dice roll
71+
//! 4
72+
//! }
73+
//!
74+
//! fn clamp_to_range(value: i32, bit_depth: u8) -> u16 {
75+
//! assert!(
76+
//! bit_depth >= 8 && bit_depth <= 16,
77+
//! "only bit depths 8-16 are supported"
78+
//! );
79+
//! let bit_depth_max = (1i32 << bit_depth) - 1;
80+
//!
81+
//! if value < 0 {
82+
//! 0
83+
//! } else if value > bit_depth_max {
84+
//! bit_depth_max as u16
85+
//! } else {
86+
//! value as u16
87+
//! }
88+
//! }
89+
//!
90+
//! pub fn change_some_rows<T: Pixel>(frame: &mut Frame<T>, bit_depth: u8) {
91+
//! for pix in frame.y_plane.pixels_mut() {
92+
//! let old_val = i32::from((*pix).into());
93+
//!
94+
//! let new_val = clamp_to_range(old_val + random_value(), bit_depth);
95+
//! if size_of::<T>() == 1 {
96+
//! *pix = T::from(new_val as u8)
97+
//! } else {
98+
//! *pix = T::try_from(new_val).expect("T is u16");
99+
//! }
100+
//! }
101+
//! }
102+
//! ```
103+
//!
104+
//! Again, `From<u8>` and `TryFrom<u16>` are the only implementations for the respective traits
105+
//! so it's not necessary to specify which one is being used.
24106
25-
use num_traits::PrimInt;
26-
use std::fmt::Debug;
107+
use core::fmt::{Binary, Debug, Display, LowerExp, LowerHex, Octal, UpperExp, UpperHex};
108+
use core::hash::Hash;
27109

28110
mod private {
29111
pub trait Sealed {}
@@ -41,6 +123,8 @@ mod private {
41123
/// data structures and algorithms to work with both standard and high bit-depth
42124
/// video content.
43125
///
126+
/// See the [module documentation][crate::pixel] for more details.
127+
///
44128
/// # Type Safety
45129
///
46130
/// The library enforces correct type usage through validation:
@@ -56,7 +140,36 @@ mod private {
56140
/// i.e. using [`std::mem::zeroed`] must __not__ cause undefined behavior for
57141
/// implementing types.
58142
pub unsafe trait Pixel:
59-
Debug + Copy + Clone + Default + Send + Sync + PrimInt + 'static + private::Sealed
143+
Sized
144+
+ Copy
145+
+ Clone
146+
// formatting
147+
+ Display
148+
+ Debug
149+
+ Octal
150+
+ LowerHex
151+
+ UpperHex
152+
+ LowerExp
153+
+ UpperExp
154+
+ Binary
155+
+ Default
156+
// comparisons
157+
+ PartialEq
158+
+ Eq
159+
+ PartialOrd
160+
+ Ord
161+
// conversions
162+
+ TryInto<u8, Error: core::error::Error>
163+
+ Into<u16>
164+
+ From<u8>
165+
+ TryFrom<u16, Error: core::error::Error>
166+
// markers
167+
+ Send
168+
+ Sync
169+
// misc.
170+
+ Hash
171+
+ 'static
172+
+ private::Sealed
60173
{
61174
}
62175

src/plane.rs

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -331,20 +331,9 @@ impl<T: Pixel> Plane<T> {
331331

332332
let total = self.width() * self.height() * byte_width;
333333
ExactSizeWrapper {
334-
iter: self.pixels().flat_map(move |pix| {
335-
let bytes: [u8; 2] = if byte_width == 1 {
336-
[
337-
pix.to_u8()
338-
.expect("Pixel::byte_data only supports u8 and u16 pixels"),
339-
0,
340-
]
341-
} else {
342-
pix.to_u16()
343-
.expect("Pixel::byte_data only supports u8 and u16 pixels")
344-
.to_le_bytes()
345-
};
346-
bytes.into_iter().take(byte_width)
347-
}),
334+
iter: self
335+
.pixels()
336+
.flat_map(move |pix| pix.into().to_le_bytes().into_iter().take(byte_width)),
348337
len: total,
349338
}
350339
}

0 commit comments

Comments
 (0)