Skip to content

Commit

Permalink
Make AVFormatContext*::streams return slice rather than iterator (#141
Browse files Browse the repository at this point in the history
)

* Add attributes support for `wrap_*` macros; Make `*Mut` & `*Ref` transparent

* Make `AVFormatContext*::streams` return slice rather than iterator
  • Loading branch information
ldm0 authored Jan 12, 2024
1 parent 9dec789 commit 57b6e28
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 98 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ jobs:
override: true

- name: Install valgrind dependencies
# On Debian and Ubuntu, libc6-dbg is required for valgrind
run: |
sudo apt-get update -qq
sudo apt-get -y install libc6-dbg
cargo install cargo-valgrind --version 2.0.2
Expand Down
126 changes: 49 additions & 77 deletions src/avformat/avformat.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{
ffi::CStr,
marker::PhantomData,
ops::Drop,
ptr::{self, NonNull},
};
Expand Down Expand Up @@ -182,13 +181,31 @@ impl AVFormatContextInput {
}

impl<'stream> AVFormatContextInput {
/// Get Iterator of all [`AVStream`]s in the [`ffi::AVFormatContext`].
pub fn streams(&'stream self) -> AVStreamRefs<'stream> {
AVStreamRefs {
stream_head: NonNull::new(self.streams as *mut _).unwrap(),
len: self.nb_streams,
_marker: PhantomData,
/// Return slice of [`AVStreamRef`].
pub fn streams(&'stream self) -> &'stream [AVStreamRef<'stream>] {
// #define `<->` as "has the same layout due to repr(transparent)"
// ```
// NonNull<ffi::AVStream> <-> *const ffi::AVStream
// AVStream <-> NonNull<ffi::AVStream>
// AVStreamRef <-> AVStream
// ```
// indicates: AVStreamRef <-> *const ffi::AVStream
let streams = self.streams as *const *const ffi::AVStream as *const AVStreamRef<'stream>;
// u32 to usize, safe
let len = self.nb_streams as usize;

// I trust that FFmpeg won't give me null pointers :-(
#[cfg(debug_assertions)]
{
let arr = unsafe {
std::slice::from_raw_parts(self.streams as *const *const ffi::AVStream, len)
};
for ptr in arr {
assert!(!ptr.is_null());
}
}

unsafe { std::slice::from_raw_parts(streams, len) }
}

/// Get [`AVInputFormatRef`] in the [`AVFormatContextInput`].
Expand Down Expand Up @@ -344,13 +361,31 @@ impl AVFormatContextOutput {
}

impl<'stream> AVFormatContextOutput {
/// Return Iterator of [`AVStreamRef`].
pub fn streams(&'stream self) -> AVStreamRefs<'stream> {
AVStreamRefs {
stream_head: NonNull::new(self.streams as *mut _).unwrap(),
len: self.nb_streams,
_marker: PhantomData,
/// Return slice of [`AVStreamRef`].
pub fn streams(&'stream self) -> &'stream [AVStreamRef<'stream>] {
// #define `<->` as "has the same layout due to repr(transparent)"
// ```
// NonNull<ffi::AVStream> <-> *const ffi::AVStream
// AVStream <-> NonNull<ffi::AVStream>
// AVStreamRef <-> AVStream
// ```
// indicates: AVStreamRef <-> *const ffi::AVStream
let streams = self.streams as *const *const ffi::AVStream as *const AVStreamRef<'stream>;
// u32 to usize, safe
let len = self.nb_streams as usize;

// I trust that FFmpeg won't give me null pointers :-(
#[cfg(debug_assertions)]
{
let arr = unsafe {
std::slice::from_raw_parts(self.streams as *const *const ffi::AVStream, len)
};
for ptr in arr {
assert!(!ptr.is_null());
}
}

unsafe { std::slice::from_raw_parts(streams, len) }
}

/// Get [`AVOutputFormat`] from the [`AVFormatContextOutput`].
Expand Down Expand Up @@ -429,7 +464,7 @@ impl AVOutputFormat {
}
}

wrap_ref_mut!(AVStream: ffi::AVStream);
wrap_ref_mut!(#[repr(transparent)] AVStream: ffi::AVStream);
settable!(AVStream {
time_base: AVRational,
});
Expand Down Expand Up @@ -505,69 +540,6 @@ impl<'stream> AVStream {
}
}

/// Iterator on reference to raw AVStream `satellite` array.
pub struct AVStreamRefsIter<'stream> {
ptr: NonNull<NonNull<ffi::AVStream>>,
end: NonNull<NonNull<ffi::AVStream>>,
_marker: PhantomData<&'stream ffi::AVStream>,
}

impl<'stream> std::iter::Iterator for AVStreamRefsIter<'stream> {
type Item = AVStreamRef<'stream>;
fn next(&mut self) -> Option<Self::Item> {
if self.ptr == self.end {
None
} else {
let old = self.ptr;
unsafe {
self.ptr = NonNull::new_unchecked(self.ptr.as_ptr().offset(1));
}
Some(unsafe { AVStreamRef::from_raw(*old.as_ref()) })
}
}
}

// ATTENTION Consider add macro for this when similar pattern occurs again.
/// A reference to raw AVStream `satellite` array, cannot be directly constructed. Using
/// this for safety concerns.
pub struct AVStreamRefs<'stream> {
stream_head: NonNull<NonNull<ffi::AVStream>>,
len: u32,
_marker: PhantomData<&'stream ffi::AVStream>,
}

impl<'stream> std::iter::IntoIterator for AVStreamRefs<'stream> {
type Item = AVStreamRef<'stream>;
type IntoIter = AVStreamRefsIter<'stream>;
fn into_iter(self) -> Self::IntoIter {
let end =
NonNull::new(unsafe { self.stream_head.as_ptr().add(self.len as usize) }).unwrap();
AVStreamRefsIter {
ptr: self.stream_head,
end,
_marker: PhantomData,
}
}
}

impl<'stream> AVStreamRefs<'stream> {
/// Get `streams[`index`]`.
pub fn get(&self, index: usize) -> Option<AVStreamRef<'stream>> {
if index < self.num() {
let stream_ptr = unsafe { *self.stream_head.as_ptr().add(index) };
Some(unsafe { AVStreamRef::from_raw(stream_ptr) })
} else {
None
}
}

/// Get `streams.len()`.
pub fn num(&self) -> usize {
// From u32 to usize, safe.
self.len as usize
}
}

#[cfg(test)]
mod test {
use super::*;
Expand Down
13 changes: 10 additions & 3 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
/// Wrapping with XXX -> XXX mapping.
macro_rules! wrap_pure {
(
$(#[$meta:meta])*
($wrapped_type: ident): $ffi_type: ty
$(,$attach: ident: $attach_type: ty = $attach_default: expr)*
) => {
$(#[$meta])*
pub struct $wrapped_type {
something_should_not_be_touched_directly: std::ptr::NonNull<$ffi_type>,
// Publicize the attachment, can be directly changed without deref_mut()
Expand Down Expand Up @@ -72,6 +74,7 @@ macro_rules! wrap_pure {
macro_rules! wrap_ref_pure {
(($wrapped_type: ident, $wrapped_ref: ident): $ffi_type: ty) => {
// This is needed for wrapping reference owned value from ffi
#[repr(transparent)]
pub struct $wrapped_ref<'a> {
inner: std::mem::ManuallyDrop<$wrapped_type>,
_marker: std::marker::PhantomData<&'a $wrapped_type>,
Expand Down Expand Up @@ -109,6 +112,7 @@ macro_rules! wrap_ref_pure {
macro_rules! wrap_mut_pure {
(($wrapped_type: ident, $wrapped_mut: ident): $ffi_type: ty) => {
// This is needed for wrapping mutable reference owned value from ffi
#[repr(transparent)]
pub struct $wrapped_mut<'a> {
inner: std::mem::ManuallyDrop<$wrapped_type>,
_marker: std::marker::PhantomData<&'a $wrapped_type>,
Expand Down Expand Up @@ -151,11 +155,12 @@ macro_rules! wrap_mut_pure {
/// Wrapping with XXXRef, XXXMut, XXX -> XXX.
macro_rules! wrap_ref_mut {
(
$(#[$meta:meta])*
$name: ident: $ffi_type: ty
$(,$attach: ident: $attach_type: ty = $attach_default: expr)* $(,)?
) => {
paste::paste! {
wrap_pure!(($name): $ffi_type $(,$attach: $attach_type = $attach_default)*);
wrap_pure!($(#[$meta])* ($name): $ffi_type $(,$attach: $attach_type = $attach_default)*);
wrap_ref_pure!(($name, [<$name Ref>]): $ffi_type);
wrap_mut_pure!(($name, [<$name Mut>]): $ffi_type);
}
Expand All @@ -165,11 +170,12 @@ macro_rules! wrap_ref_mut {
/// Wrapping with XXXRef, XXX -> XXX.
macro_rules! wrap_ref {
(
$(#[$meta:meta])*
$name: ident: $ffi_type: ty
$(,$attach: ident: $attach_type: ty = $attach_default: expr)* $(,)?
) => {
paste::paste! {
wrap_pure!(($name): $ffi_type $(,$attach: $attach_type = $attach_default)*);
wrap_pure!($(#[$meta])* ($name): $ffi_type $(,$attach: $attach_type = $attach_default)*);
wrap_ref_pure!(($name, [<$name Ref>]): $ffi_type);
}
};
Expand All @@ -178,11 +184,12 @@ macro_rules! wrap_ref {
/// Wrapping with XXXMut, XXX -> XXX.
macro_rules! wrap_mut {
(
$(#[$meta:meta])*
$name: ident: $ffi_type: ty
$(,$attach: ident: $attach_type: ty = $attach_default: expr)* $(,)?
) => {
paste::paste! {
wrap_pure!(($name): $ffi_type $(,$attach: $attach_type = $attach_default)*);
wrap_pure!($(#[$meta])* ($name): $ffi_type $(,$attach: $attach_type = $attach_default)*);
wrap_mut_pure!(($name, [<$name Mut>]): $ffi_type);
}
};
Expand Down
6 changes: 1 addition & 5 deletions tests/avio_writing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,7 @@ fn encode_write_frame(
packet.set_stream_index(out_stream_index as i32);
packet.rescale_ts(
encode_context.time_base,
output_format_context
.streams()
.get(out_stream_index)
.unwrap()
.time_base,
output_format_context.streams()[out_stream_index].time_base,
);

match output_format_context.interleaved_write_frame(&mut packet) {
Expand Down
10 changes: 2 additions & 8 deletions tests/remux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,8 @@ fn remux(input_path: &CStr, output_path: &CStr) -> Result<()> {
continue;
};
{
let input_stream = input_format_context
.streams()
.get(input_stream_index)
.unwrap();
let output_stream = output_format_context
.streams()
.get(output_stream_index)
.unwrap();
let input_stream = &input_format_context.streams()[input_stream_index];
let output_stream = &output_format_context.streams()[output_stream_index];
log_packet(input_stream.time_base, &packet, "in");
packet.rescale_ts(input_stream.time_base, output_stream.time_base);
packet.set_stream_index(output_stream_index as i32);
Expand Down
6 changes: 1 addition & 5 deletions tests/transcoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,7 @@ fn encode_write_frame(
packet.set_stream_index(out_stream_index as i32);
packet.rescale_ts(
encode_context.time_base,
output_format_context
.streams()
.get(out_stream_index)
.unwrap()
.time_base,
output_format_context.streams()[out_stream_index].time_base,
);

match output_format_context.interleaved_write_frame(&mut packet) {
Expand Down

0 comments on commit 57b6e28

Please sign in to comment.