From 23dde387b761417a10e285d34e3b87a1033870a6 Mon Sep 17 00:00:00 2001 From: tilpner Date: Tue, 29 Aug 2023 15:47:54 +0200 Subject: [PATCH 1/5] feat(util/rational): add `Rational::non_zero` for guarding against zero-valued Rationals --- src/util/rational.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/util/rational.rs b/src/util/rational.rs index 9c9b89e..9b51202 100644 --- a/src/util/rational.rs +++ b/src/util/rational.rs @@ -14,6 +14,8 @@ use crate::ffi::*; pub struct Rational(pub i32, pub i32); impl Rational { + pub const ZERO: Rational = Rational(0, 1); + #[inline] pub fn new(numerator: i32, denominator: i32) -> Self { Rational(numerator, denominator) @@ -69,6 +71,17 @@ impl Rational { pub fn approx(&self) -> f64 { self.0 as f64 / self.1 as f64 } + + #[inline] + pub fn non_zero(self) -> Option { + (self.numerator() != 0).then_some(self) + } +} + +impl Default for Rational { + fn default() -> Self { + Rational::ZERO + } } impl From for Rational { From df08039eeb6f1600604b3b38a0e02056a1b9330d Mon Sep 17 00:00:00 2001 From: tilpner Date: Tue, 29 Aug 2023 15:48:06 +0200 Subject: [PATCH 2/5] refactor: use `Rational::non_zero` instead of manual comparisons --- src/codec/context.rs | 18 ++---------------- src/codec/decoder/opened.rs | 11 +---------- src/codec/video.rs | 9 ++------- src/format/stream/stream.rs | 7 +------ 4 files changed, 6 insertions(+), 39 deletions(-) diff --git a/src/codec/context.rs b/src/codec/context.rs index a7a323b..41dcae7 100644 --- a/src/codec/context.rs +++ b/src/codec/context.rs @@ -136,26 +136,12 @@ impl Context { } pub fn frame_rate(&self) -> Option { - unsafe { - let fr = Rational::from((*self.as_ptr()).framerate); - if fr == Rational(0, 1) { - None - } - else { - Some(fr) - } - } + unsafe { Rational::from((*self.as_ptr()).framerate).non_zero() } } pub fn set_frame_rate>(&mut self, value: Option) { unsafe { - if let Some(value) = value { - (*self.as_mut_ptr()).framerate = value.into().into(); - } - else { - (*self.as_mut_ptr()).framerate.num = 0; - (*self.as_mut_ptr()).framerate.den = 1; - } + (*self.as_mut_ptr()).framerate = value.map(Into::into).unwrap_or(Rational::ZERO).into(); } } } diff --git a/src/codec/decoder/opened.rs b/src/codec/decoder/opened.rs index eb7fc8b..2413c7e 100644 --- a/src/codec/decoder/opened.rs +++ b/src/codec/decoder/opened.rs @@ -82,16 +82,7 @@ impl Opened { } pub fn frame_rate(&self) -> Option { - unsafe { - let value = (*self.as_ptr()).framerate; - - if value == (AVRational { num: 0, den: 1 }) { - None - } - else { - Some(Rational::from(value)) - } - } + unsafe { Rational::from((*self.as_ptr()).framerate).non_zero() } } pub fn flush(&mut self) { diff --git a/src/codec/video.rs b/src/codec/video.rs index fbb09a1..6e13463 100644 --- a/src/codec/video.rs +++ b/src/codec/video.rs @@ -61,14 +61,9 @@ impl Iterator for RateIter { fn next(&mut self) -> Option<::Item> { unsafe { - if (*self.ptr).num == 0 && (*self.ptr).den == 0 { - return None; - } - - let rate = (*self.ptr).into(); + let rate = Rational::from(*self.ptr).non_zero(); self.ptr = self.ptr.offset(1); - - Some(rate) + rate } } } diff --git a/src/format/stream/stream.rs b/src/format/stream/stream.rs index b8c7c74..60bbd5f 100644 --- a/src/format/stream/stream.rs +++ b/src/format/stream/stream.rs @@ -107,12 +107,7 @@ impl<'a> Stream<'a> { ptr::null_mut(), )); - if r == Rational(0, 1) { - None - } - else { - Some(r) - } + r.non_zero() } } From 3d6efeba5eb41f48ab9213a3e4d4f75eb9928dba Mon Sep 17 00:00:00 2001 From: tilpner Date: Tue, 29 Aug 2023 15:45:33 +0200 Subject: [PATCH 3/5] fix(codec, format)!: detect unset `time_base`s, allow unsetting `time_base` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of silently passing possibly unset `time_base`s back into ffmpeg, force users to explicitly handle (even if just by early panic) their absence. BREAKING CHANGE: `time_base` and `set_time_base` functions now return/accept Option (±Into). `unwrap`/`expect` is a valid way to surface this unwanted state early in user code. For symmetry, `set_time_base` also allows unsetting by passing a `None`, so all existing calls (with non-zero timebases) need to wrap the argument in `Some`. --- src/codec/context.rs | 8 ++++---- src/format/chapter/chapter.rs | 4 ++-- src/format/stream/stream.rs | 4 ++-- src/format/stream/stream_mut.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/codec/context.rs b/src/codec/context.rs index 41dcae7..58448b3 100644 --- a/src/codec/context.rs +++ b/src/codec/context.rs @@ -125,13 +125,13 @@ impl Context { Parameters::from(self) } - pub fn time_base(&self) -> Rational { - unsafe { Rational::from((*self.as_ptr()).time_base) } + pub fn time_base(&self) -> Option { + unsafe { Rational::from((*self.as_ptr()).time_base).non_zero() } } - pub fn set_time_base>(&mut self, value: R) { + pub fn set_time_base>(&mut self, value: Option) { unsafe { - (*self.as_mut_ptr()).time_base = value.into().into(); + (*self.as_mut_ptr()).time_base = value.map(Into::into).unwrap_or(Rational::ZERO).into(); } } diff --git a/src/format/chapter/chapter.rs b/src/format/chapter/chapter.rs index d5f7d56..00f19cd 100644 --- a/src/format/chapter/chapter.rs +++ b/src/format/chapter/chapter.rs @@ -26,8 +26,8 @@ impl<'a> Chapter<'a> { unsafe { (*self.as_ptr()).id } } - pub fn time_base(&self) -> Rational { - unsafe { Rational::from((*self.as_ptr()).time_base) } + pub fn time_base(&self) -> Option { + unsafe { Rational::from((*self.as_ptr()).time_base).non_zero() } } pub fn start(&self) -> i64 { diff --git a/src/format/stream/stream.rs b/src/format/stream/stream.rs index 60bbd5f..b4f9114 100644 --- a/src/format/stream/stream.rs +++ b/src/format/stream/stream.rs @@ -53,8 +53,8 @@ impl<'a> Stream<'a> { unsafe { (*self.as_ptr()).index as usize } } - pub fn time_base(&self) -> Rational { - unsafe { Rational::from((*self.as_ptr()).time_base) } + pub fn time_base(&self) -> Option { + unsafe { Rational::from((*self.as_ptr()).time_base).non_zero() } } pub fn start_time(&self) -> Option { diff --git a/src/format/stream/stream_mut.rs b/src/format/stream/stream_mut.rs index 077cf64..902df39 100644 --- a/src/format/stream/stream_mut.rs +++ b/src/format/stream/stream_mut.rs @@ -26,9 +26,9 @@ impl<'a> StreamMut<'a> { } impl<'a> StreamMut<'a> { - pub fn set_time_base>(&mut self, value: R) { + pub fn set_time_base>(&mut self, value: Option) { unsafe { - (*self.as_mut_ptr()).time_base = value.into().into(); + (*self.as_mut_ptr()).time_base = value.map(Into::into).unwrap_or(Rational::ZERO).into(); } } From 825e0d448419870859b28bfbf7365a987d05875f Mon Sep 17 00:00:00 2001 From: tilpner Date: Tue, 29 Aug 2023 14:00:18 +0200 Subject: [PATCH 4/5] feat(codec/packet, util/frame): expose `time_base` fields --- src/codec/packet/packet.rs | 10 ++++++++++ src/util/frame/mod.rs | 14 +++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/codec/packet/packet.rs b/src/codec/packet/packet.rs index 7b36c90..b3a9e3e 100644 --- a/src/codec/packet/packet.rs +++ b/src/codec/packet/packet.rs @@ -106,6 +106,16 @@ impl Packet { self.0.stream_index = index as c_int; } + #[inline] + pub fn time_base(&self) -> Option { + Rational::from(self.0.time_base).non_zero() + } + + #[inline] + pub fn set_time_base(&mut self, time_base: Option>) { + self.0.time_base = time_base.map(Into::into).unwrap_or(Rational::ZERO).into(); + } + #[inline] pub fn pts(&self) -> Option { match self.0.pts { diff --git a/src/util/frame/mod.rs b/src/util/frame/mod.rs index 49bf1d9..1d07739 100644 --- a/src/util/frame/mod.rs +++ b/src/util/frame/mod.rs @@ -10,7 +10,7 @@ pub use self::audio::Audio; pub mod flag; pub use self::flag::Flags; -use crate::{ffi::*, Dictionary, DictionaryRef, Error}; +use crate::{ffi::*, Dictionary, DictionaryRef, Error, Rational}; #[derive(PartialEq, Eq, Copy, Clone, Debug)] pub struct Packet { @@ -102,6 +102,18 @@ impl Frame { } } + #[inline] + pub fn time_base(&self) -> Option { + unsafe { Rational::from((*self.as_ptr()).time_base).non_zero() } + } + + #[inline] + pub fn set_time_base(&mut self, time_base: Option>) { + unsafe { + (*self.as_mut_ptr()).time_base = time_base.map(Into::into).unwrap_or(Rational::ZERO).into(); + } + } + #[inline] pub fn pts(&self) -> Option { unsafe { From 49db1407b6427baf5877939abe6bf8db8b26f990 Mon Sep 17 00:00:00 2001 From: tilpner Date: Tue, 29 Aug 2023 15:44:17 +0200 Subject: [PATCH 5/5] fix(examples): adjust for time_base changes --- examples/chapters.rs | 6 +++--- examples/metadata.rs | 7 +++++-- examples/remux.rs | 6 +++--- examples/transcode-audio.rs | 13 +++++++------ examples/transcode-x264.rs | 12 ++++++------ 5 files changed, 24 insertions(+), 20 deletions(-) diff --git a/examples/chapters.rs b/examples/chapters.rs index 4df063c..d83863d 100644 --- a/examples/chapters.rs +++ b/examples/chapters.rs @@ -9,7 +9,7 @@ fn main() { for chapter in ictx.chapters() { println!("chapter id {}:", chapter.id()); - println!("\ttime_base: {}", chapter.time_base()); + println!("\ttime_base: {:?}", chapter.time_base()); println!("\tstart: {}", chapter.start()); println!("\tend: {}", chapter.end()); @@ -28,7 +28,7 @@ fn main() { match octx.add_chapter( chapter.id(), - chapter.time_base(), + chapter.time_base().expect("Input chapter is missing a time base"), chapter.start(), chapter.end(), &title, @@ -43,7 +43,7 @@ fn main() { println!("\nOuput: nb chapters: {}", octx.nb_chapters()); for chapter in octx.chapters() { println!("chapter id {}:", chapter.id()); - println!("\ttime_base: {}", chapter.time_base()); + println!("\ttime_base: {:?}", chapter.time_base()); println!("\tstart: {}", chapter.start()); println!("\tend: {}", chapter.end()); for (k, v) in chapter.metadata().iter() { diff --git a/examples/metadata.rs b/examples/metadata.rs index 9f60110..279e7c3 100644 --- a/examples/metadata.rs +++ b/examples/metadata.rs @@ -32,12 +32,15 @@ fn main() { for stream in context.streams() { println!("stream index {}:", stream.index()); - println!("\ttime_base: {}", stream.time_base()); + println!("\ttime_base: {:?}", stream.time_base()); println!("\tstart_time: {:?}", stream.start_time()); println!("\tduration (stream timebase): {:?}", stream.duration()); println!( "\tduration (seconds): {:?}", - stream.duration().map(|d| d as f64 * f64::from(stream.time_base())) + stream + .time_base() + .zip(stream.duration()) + .map(|(tb, d)| d as f64 * f64::from(tb)) ); println!("\tframes: {}", stream.frames()); println!("\tdisposition: {:?}", stream.disposition()); diff --git a/examples/remux.rs b/examples/remux.rs index d3c5edd..b79f1da 100644 --- a/examples/remux.rs +++ b/examples/remux.rs @@ -1,6 +1,6 @@ use std::env; -use ffmpeg::{format, media, Rational}; +use ffmpeg::{format, media}; fn main() { let input_file = env::args().nth(1).expect("missing input file"); @@ -12,7 +12,7 @@ fn main() { let mut octx = format::output(&output_file).unwrap(); let mut stream_mapping = vec![0; ictx.nb_streams() as _]; - let mut ist_time_bases = vec![Rational(0, 1); ictx.nb_streams() as _]; + let mut ist_time_bases = vec![None; ictx.nb_streams() as _]; let mut ost_index = 0; for (ist_index, ist) in ictx.streams().enumerate() { let codec_par = ist.parameters(); @@ -45,7 +45,7 @@ fn main() { continue; } let ost = octx.stream(ost_index as _).unwrap(); - packet.rescale_ts(ist_time_bases[ist_index], ost.time_base()); + packet.rescale_ts(ist_time_bases[ist_index].unwrap(), ost.time_base().unwrap()); packet.set_position(-1); packet.set_stream(ost_index as _); packet.write_interleaved(&mut octx).unwrap(); diff --git a/examples/transcode-audio.rs b/examples/transcode-audio.rs index 8f71b7c..9e578ce 100644 --- a/examples/transcode-audio.rs +++ b/examples/transcode-audio.rs @@ -11,7 +11,7 @@ fn filter( let args = format!( "time_base={}:sample_rate={}:sample_fmt={}:channel_layout={}", - decoder.time_base(), + decoder.time_base().unwrap(), decoder.sample_rate(), decoder.format().name(), decoder.channel_layout().describe().unwrap() @@ -94,16 +94,17 @@ fn transcoder>( encoder.set_bit_rate(decoder.bit_rate()); encoder.set_max_bit_rate(decoder.max_bit_rate()); - encoder.set_time_base((1, decoder.sample_rate() as i32)); - output.set_time_base((1, decoder.sample_rate() as i32)); + let enc_tb = (1, decoder.sample_rate() as i32); + encoder.set_time_base(Some(enc_tb)); + output.set_time_base(Some(enc_tb)); let encoder = encoder.open_as(codec)?; output.set_parameters(encoder.parameters()); let filter = filter(filter_spec, &decoder, &encoder)?; - let in_time_base = decoder.time_base(); - let out_time_base = output.time_base(); + let in_time_base = decoder.time_base().unwrap(); + let out_time_base = output.time_base().unwrap(); Ok(Transcoder { stream: input.index(), @@ -207,7 +208,7 @@ fn main() { for res in ictx.packets() { let (stream, mut packet) = res.unwrap(); if stream.index() == transcoder.stream { - packet.rescale_ts(stream.time_base(), transcoder.in_time_base); + packet.rescale_ts(stream.time_base().unwrap(), transcoder.in_time_base); transcoder.send_packet_to_decoder(&packet); transcoder.receive_and_process_decoded_frames(&mut octx); } diff --git a/examples/transcode-x264.rs b/examples/transcode-x264.rs index b49788f..88ae111 100644 --- a/examples/transcode-x264.rs +++ b/examples/transcode-x264.rs @@ -51,7 +51,7 @@ impl Transcoder { encoder.set_aspect_ratio(decoder.aspect_ratio()); encoder.set_format(decoder.format()); encoder.set_frame_rate(decoder.frame_rate()); - encoder.set_time_base(decoder.frame_rate().unwrap().invert()); + encoder.set_time_base(Some(decoder.frame_rate().unwrap().invert())); if global_header { encoder.set_flags(codec::Flags::GLOBAL_HEADER); } @@ -85,7 +85,7 @@ impl Transcoder { self.frame_count += 1; let timestamp = frame.timestamp(); self.log_progress(f64::from( - Rational(timestamp.unwrap_or(0) as i32, 1) * self.decoder.time_base(), + Rational(timestamp.unwrap_or(0) as i32, 1) * self.decoder.time_base().unwrap(), )); frame.set_pts(timestamp); frame.set_kind(picture::Type::None); @@ -106,7 +106,7 @@ impl Transcoder { let mut encoded = Packet::empty(); while self.encoder.receive_packet(&mut encoded).is_ok() { encoded.set_stream(self.ost_index); - encoded.rescale_ts(self.decoder.time_base(), ost_time_base); + encoded.rescale_ts(self.decoder.time_base().unwrap(), ost_time_base); encoded.write_interleaved(octx).unwrap(); } } @@ -170,7 +170,7 @@ fn main() { continue; } stream_mapping[ist_index] = ost_index; - ist_time_bases[ist_index] = ist.time_base(); + ist_time_bases[ist_index] = ist.time_base().unwrap(); if ist_medium == media::Type::Video { // Initialize transcoder for video stream. transcoders.insert( @@ -204,7 +204,7 @@ fn main() { octx.write_header().unwrap(); for (ost_index, _) in octx.streams().enumerate() { - ost_time_bases[ost_index] = octx.stream(ost_index as _).unwrap().time_base(); + ost_time_bases[ost_index] = octx.stream(ost_index as _).unwrap().time_base().unwrap(); } for res in ictx.packets() { @@ -217,7 +217,7 @@ fn main() { let ost_time_base = ost_time_bases[ost_index as usize]; match transcoders.get_mut(&ist_index) { Some(transcoder) => { - packet.rescale_ts(stream.time_base(), transcoder.decoder.time_base()); + packet.rescale_ts(stream.time_base().unwrap(), transcoder.decoder.time_base().unwrap()); transcoder.send_packet_to_decoder(&packet); transcoder.receive_and_process_decoded_frames(&mut octx, ost_time_base); }