[ENG-1039] Apply hard clippy lints to the entire ffmpeg crate (#1281)

* apply clippy lints to the entire ffmpeg crate in hopes of making it safer

* wording

* revert the prisma changes

* fix typo

Co-authored-by: Oscar Beaumont <oscar@otbeaumont.me>

* fix another typo

Co-authored-by: Oscar Beaumont <oscar@otbeaumont.me>

* remove so many `#[must_use]`s

* fix bad merge and hopefully clippy

* clippy please work i beg

* make HEIF_EXTENSIONS always available

---------

Co-authored-by: Oscar Beaumont <oscar@otbeaumont.me>
Co-authored-by: Brendan Allan <brendonovich@outlook.com>

core/src/api/nodes.rs

@@ -16,8 +16,6 @@ pub(crate) fn mount() -> AlphaRouter<Ctx> {
 			pub struct ChangeNodeNameArgs {
 				pub name: Option<String>,
 			}
-			// TODO: validate name isn't empty or too long
-
 			R.mutation(|node, args: ChangeNodeNameArgs| async move {
 				if let Some(name) = args.name {
 					if name.is_empty() || name.len() > 32 {

core/src/object/media/thumbnail/mod.rs

@@ -143,6 +143,11 @@ pub async fn generate_image_thumbnail<P: AsRef<Path>>(
 		return Err(ThumbnailerError::TooLarge.into());
 	}
 
+	#[cfg(all(feature = "heif", not(target_os = "linux")))]
+	if metadata.len() > sd_heif::MAXIMUM_FILE_SIZE && HEIF_EXTENSIONS.contains(ext) {
+		return Err(ThumbnailerError::TooLarge.into());
+	}
+
 	let data = Arc::new(
 		fs::read(file_path)
 			.await
@@ -223,7 +228,7 @@ pub async fn generate_image_thumbnail<P: AsRef<Path>>(
 }
 
 #[cfg(feature = "ffmpeg")]
-pub async fn generate_video_thumbnail<P: AsRef<Path>>(
+pub async fn generate_video_thumbnail<P: AsRef<Path> + Send>(
 	file_path: P,
 	output_path: P,
 ) -> Result<(), Box<dyn Error>> {

crates/ffmpeg/src/error.rs

@@ -1,5 +1,5 @@
-use std::ffi::c_int;
 use std::path::PathBuf;
+use std::{ffi::c_int, num::TryFromIntError};
 use thiserror::Error;
 use tokio::task::JoinError;
 
@@ -38,11 +38,13 @@ pub enum ThumbnailerError {
 	BackgroundTaskFailed(#[from] JoinError),
 	#[error("The video is most likely corrupt and will be skipped")]
 	CorruptVideo,
+	#[error("Error while casting an integer to another integer type")]
+	IntCastError(#[from] TryFromIntError),
 }
 
-/// Enum to represent possible errors from FFmpeg library
+/// Enum to represent possible errors from `FFmpeg` library
 ///
-/// Extracted from https://ffmpeg.org/doxygen/trunk/group__lavu__error.html
+/// Extracted from <https://ffmpeg.org/doxygen/trunk/group__lavu__error.html>
 #[derive(Error, Debug)]
 pub enum FfmpegError {
 	#[error("Bitstream filter not found")]
@@ -114,31 +116,31 @@ pub enum FfmpegError {
 impl From<c_int> for FfmpegError {
 	fn from(code: c_int) -> Self {
 		match code {
-			AVERROR_BSF_NOT_FOUND => FfmpegError::BitstreamFilterNotFound,
-			AVERROR_BUG => FfmpegError::InternalBug,
-			AVERROR_BUFFER_TOO_SMALL => FfmpegError::BufferTooSmall,
-			AVERROR_DECODER_NOT_FOUND => FfmpegError::DecoderNotFound,
-			AVERROR_DEMUXER_NOT_FOUND => FfmpegError::DemuxerNotFound,
-			AVERROR_ENCODER_NOT_FOUND => FfmpegError::EncoderNotFound,
-			AVERROR_EOF => FfmpegError::Eof,
-			AVERROR_EXIT => FfmpegError::Exit,
-			AVERROR_EXTERNAL => FfmpegError::External,
-			AVERROR_FILTER_NOT_FOUND => FfmpegError::FilterNotFound,
-			AVERROR_INVALIDDATA => FfmpegError::InvalidData,
-			AVERROR_MUXER_NOT_FOUND => FfmpegError::MuxerNotFound,
-			AVERROR_OPTION_NOT_FOUND => FfmpegError::OptionNotFound,
-			AVERROR_PATCHWELCOME => FfmpegError::NotImplemented,
-			AVERROR_PROTOCOL_NOT_FOUND => FfmpegError::ProtocolNotFound,
-			AVERROR_STREAM_NOT_FOUND => FfmpegError::StreamNotFound,
-			AVERROR_BUG2 => FfmpegError::InternalBug2,
-			AVERROR_UNKNOWN => FfmpegError::Unknown,
-			AVERROR_HTTP_BAD_REQUEST => FfmpegError::HttpBadRequest,
-			AVERROR_HTTP_UNAUTHORIZED => FfmpegError::HttpUnauthorized,
-			AVERROR_HTTP_FORBIDDEN => FfmpegError::HttpForbidden,
-			AVERROR_HTTP_NOT_FOUND => FfmpegError::HttpNotFound,
-			AVERROR_HTTP_OTHER_4XX => FfmpegError::HttpOther4xx,
-			AVERROR_HTTP_SERVER_ERROR => FfmpegError::HttpServerError,
-			other => FfmpegError::OtherOSError(AVUNERROR(other)),
+			AVERROR_BSF_NOT_FOUND => Self::BitstreamFilterNotFound,
+			AVERROR_BUG => Self::InternalBug,
+			AVERROR_BUFFER_TOO_SMALL => Self::BufferTooSmall,
+			AVERROR_DECODER_NOT_FOUND => Self::DecoderNotFound,
+			AVERROR_DEMUXER_NOT_FOUND => Self::DemuxerNotFound,
+			AVERROR_ENCODER_NOT_FOUND => Self::EncoderNotFound,
+			AVERROR_EOF => Self::Eof,
+			AVERROR_EXIT => Self::Exit,
+			AVERROR_EXTERNAL => Self::External,
+			AVERROR_FILTER_NOT_FOUND => Self::FilterNotFound,
+			AVERROR_INVALIDDATA => Self::InvalidData,
+			AVERROR_MUXER_NOT_FOUND => Self::MuxerNotFound,
+			AVERROR_OPTION_NOT_FOUND => Self::OptionNotFound,
+			AVERROR_PATCHWELCOME => Self::NotImplemented,
+			AVERROR_PROTOCOL_NOT_FOUND => Self::ProtocolNotFound,
+			AVERROR_STREAM_NOT_FOUND => Self::StreamNotFound,
+			AVERROR_BUG2 => Self::InternalBug2,
+			AVERROR_UNKNOWN => Self::Unknown,
+			AVERROR_HTTP_BAD_REQUEST => Self::HttpBadRequest,
+			AVERROR_HTTP_UNAUTHORIZED => Self::HttpUnauthorized,
+			AVERROR_HTTP_FORBIDDEN => Self::HttpForbidden,
+			AVERROR_HTTP_NOT_FOUND => Self::HttpNotFound,
+			AVERROR_HTTP_OTHER_4XX => Self::HttpOther4xx,
+			AVERROR_HTTP_SERVER_ERROR => Self::HttpServerError,
+			other => Self::OtherOSError(AVUNERROR(other)),
 		}
 	}
 }

crates/ffmpeg/src/film_strip.rs

@@ -625,7 +625,7 @@ struct FilmStrip {
 	strip: Option<&'static [u8]>,
 }
 
-pub(crate) fn film_strip_filter(video_frame: &mut VideoFrame) {
+pub fn film_strip_filter(video_frame: &mut VideoFrame) {
 	let FilmStrip {
 		width,
 		height,

crates/ffmpeg/src/lib.rs

@@ -18,8 +18,8 @@ pub use thumbnailer::{Thumbnailer, ThumbnailerBuilder};
 
 /// Helper function to generate a thumbnail file from a video file with reasonable defaults
 pub async fn to_thumbnail(
-	video_file_path: impl AsRef<Path>,
-	output_thumbnail_path: impl AsRef<Path>,
+	video_file_path: impl AsRef<Path> + Send,
+	output_thumbnail_path: impl AsRef<Path> + Send,
 	size: u32,
 	quality: f32,
 ) -> Result<(), ThumbnailerError> {
@@ -34,7 +34,7 @@ pub async fn to_thumbnail(
 
 /// Helper function to generate a thumbnail bytes from a video file with reasonable defaults
 pub async fn to_webp_bytes(
-	video_file_path: impl AsRef<Path>,
+	video_file_path: impl AsRef<Path> + Send,
 	size: u32,
 	quality: f32,
 ) -> Result<Vec<u8>, ThumbnailerError> {

crates/ffmpeg/src/movie_decoder.rs

@@ -24,12 +24,12 @@ use std::{
 };
 
 #[derive(Debug, Clone, Copy)]
-pub(crate) enum ThumbnailSize {
+pub enum ThumbnailSize {
 	Dimensions { width: u32, height: u32 },
 	Size(u32),
 }
 
-pub(crate) struct MovieDecoder {
+pub struct MovieDecoder {
 	video_stream_index: i32,
 	format_context: *mut AVFormatContext,
 	video_codec_context: *mut AVCodecContext,
@@ -130,7 +130,7 @@ impl MovieDecoder {
 		Ok(())
 	}
 
-	pub(crate) fn embedded_metadata_is_available(&self) -> bool {
+	pub(crate) const fn embedded_metadata_is_available(&self) -> bool {
 		self.use_embedded_data
 	}
 
@@ -139,7 +139,7 @@ impl MovieDecoder {
 			return Ok(());
 		}
 
-		let timestamp = (AV_TIME_BASE as i64).checked_mul(seconds).unwrap_or(0);
+		let timestamp = i64::from(AV_TIME_BASE).checked_mul(seconds).unwrap_or(0);
 
 		check_error(
 			unsafe { av_seek_frame(self.format_context, -1, timestamp, 0) },
@@ -217,9 +217,13 @@ impl MovieDecoder {
 			));
 		}
 
-		video_frame.width = unsafe { (*new_frame.as_mut_ptr()).width as u32 };
-		video_frame.height = unsafe { (*new_frame.as_mut_ptr()).height as u32 };
-		video_frame.line_size = unsafe { (*new_frame.as_mut_ptr()).linesize[0] as u32 };
+		// SAFETY: these should always be positive, so clippy doesn't need to alert on them
+		#[allow(clippy::cast_sign_loss)]
+		{
+			video_frame.width = unsafe { (*new_frame.as_mut_ptr()).width as u32 };
+			video_frame.height = unsafe { (*new_frame.as_mut_ptr()).height as u32 };
+			video_frame.line_size = unsafe { (*new_frame.as_mut_ptr()).linesize[0] as u32 };
+		}
 		video_frame.source = if self.use_embedded_data {
 			Some(FrameSource::Metadata)
 		} else {
@@ -256,7 +260,9 @@ impl MovieDecoder {
 		Ok(())
 	}
 
-	pub(crate) fn get_video_duration(&self) -> Duration {
+	// SAFETY: this should always be positive, so clippy doesn't need to alert on them
+	#[allow(clippy::cast_sign_loss)]
+	pub fn get_video_duration(&self) -> Duration {
 		Duration::from_secs(unsafe { (*self.format_context).duration as u64 / AV_TIME_BASE as u64 })
 	}
 
@@ -311,7 +317,7 @@ impl MovieDecoder {
 		let mut embedded_data_streams = vec![];
 		let empty_cstring = CString::new("").unwrap();
 
-		for stream_idx in 0..(unsafe { (*self.format_context).nb_streams as i32 }) {
+		for stream_idx in 0..(unsafe { (*self.format_context).nb_streams.try_into()? }) {
 			let stream = unsafe { *(*self.format_context).streams.offset(stream_idx as isize) };
 			let codec_params = unsafe { (*stream).codecpar };
 
@@ -425,6 +431,7 @@ impl MovieDecoder {
 		}
 	}
 
+	#[allow(clippy::too_many_lines)]
 	fn initialize_filter_graph(
 		&mut self,
 		timebase: &AVRational,
@@ -483,7 +490,7 @@ impl MovieDecoder {
 			&mut scale_filter,
 			"scale",
 			"thumb_scale",
-			&self.create_scale_string(scaled_size, maintain_aspect_ratio)?,
+			&Self::create_scale_string(scaled_size, maintain_aspect_ratio),
 			self.filter_graph,
 			"Failed to create scale filter",
 		)?;
@@ -523,10 +530,10 @@ impl MovieDecoder {
 		check_error(
 			unsafe {
 				avfilter_link(
-					if !rotate_filter.is_null() {
-						rotate_filter
-					} else {
+					if rotate_filter.is_null() {
 						format_filter
+					} else {
+						rotate_filter
 					},
 					0,
 					self.filter_sink,
@@ -560,10 +567,10 @@ impl MovieDecoder {
 				avfilter_link(
 					self.filter_source,
 					0,
-					if !yadif_filter.is_null() {
-						yadif_filter
-					} else {
+					if yadif_filter.is_null() {
 						scale_filter
+					} else {
+						yadif_filter
 					},
 					0,
 				)
@@ -579,18 +586,15 @@ impl MovieDecoder {
 		Ok(())
 	}
 
-	fn create_scale_string(
-		&self,
-		size: Option<ThumbnailSize>,
-		maintain_aspect_ratio: bool,
-	) -> Result<String, ThumbnailerError> {
+	fn create_scale_string(size: Option<ThumbnailSize>, maintain_aspect_ratio: bool) -> String {
 		let mut scaled_width;
 		let mut scaled_height = -1;
 		if size.is_none() {
-			return Ok("w=0:h=0".to_string());
+			return "w=0:h=0".to_string();
 		}
 		let size = size.expect("Size should have been checked for None");
 
+		#[allow(clippy::cast_possible_wrap)]
 		match size {
 			ThumbnailSize::Dimensions { width, height } => {
 				scaled_width = width as i32;
@@ -602,11 +606,11 @@ impl MovieDecoder {
 		}
 
 		if scaled_width <= 0 {
-			scaled_width = -1
+			scaled_width = -1;
 		}
 
 		if scaled_height <= 0 {
-			scaled_height = -1
+			scaled_height = -1;
 		}
 
 		let mut scale = String::new();
@@ -621,9 +625,10 @@ impl MovieDecoder {
 
 		// TODO: Handle anamorphic videos
 
-		Ok(scale)
+		scale
 	}
 
+	#[allow(clippy::cast_ptr_alignment)]
 	fn get_stream_rotation(&self) -> i32 {
 		let matrix = unsafe {
 			av_stream_get_side_data(

crates/ffmpeg/src/thumbnailer.rs

@@ -16,19 +16,17 @@ impl Thumbnailer {
 	/// Processes an video input file and write to file system a thumbnail with webp format
 	pub async fn process(
 		&self,
-		video_file_path: impl AsRef<Path>,
-		output_thumbnail_path: impl AsRef<Path>,
+		video_file_path: impl AsRef<Path> + Send,
+		output_thumbnail_path: impl AsRef<Path> + Send,
 	) -> Result<(), ThumbnailerError> {
-		fs::create_dir_all(
-			output_thumbnail_path
-				.as_ref()
-				.parent()
-				.ok_or(io::Error::new(
-					io::ErrorKind::InvalidInput,
-					"Cannot determine parent directory",
-				))?,
-		)
-		.await?;
+		let path = output_thumbnail_path.as_ref().parent().ok_or_else(|| {
+			io::Error::new(
+				io::ErrorKind::InvalidInput,
+				"Cannot determine parent directory",
+			)
+		})?;
+
+		fs::create_dir_all(path).await?;
 
 		fs::write(
 			output_thumbnail_path,
@@ -41,7 +39,7 @@ impl Thumbnailer {
 	/// Processes an video input file and returns a webp encoded thumbnail as bytes
 	pub async fn process_to_webp_bytes(
 		&self,
-		video_file_path: impl AsRef<Path>,
+		video_file_path: impl AsRef<Path> + Send,
 	) -> Result<Vec<u8>, ThumbnailerError> {
 		let video_file_path = video_file_path.as_ref().to_path_buf();
 		let prefer_embedded_metadata = self.builder.prefer_embedded_metadata;
@@ -56,10 +54,12 @@ impl Thumbnailer {
 			// We actually have to decode a frame to get some metadata before we can start decoding for real
 			decoder.decode_video_frame()?;
 
+			#[allow(clippy::cast_possible_truncation)]
+			#[allow(clippy::cast_precision_loss)]
 			if !decoder.embedded_metadata_is_available() {
 				let result = decoder.seek(
-					(decoder.get_video_duration().as_secs() as f32 * seek_percentage).round()
-						as i64,
+					(decoder.get_video_duration().as_secs() as f64 * f64::from(seek_percentage))
+						.round() as i64,
 				);
 
 				if let Err(err) = result {
@@ -95,6 +95,7 @@ impl Thumbnailer {
 /// `ThumbnailerBuilder` struct holds data to build a `Thumbnailer` struct, exposing many methods
 /// to configure how a thumbnail must be generated.
 #[derive(Debug, Clone)]
+#[must_use]
 pub struct ThumbnailerBuilder {
 	maintain_aspect_ratio: bool,
 	size: ThumbnailSize,
@@ -126,23 +127,23 @@ impl ThumbnailerBuilder {
 	/// - `prefer_embedded_metadata`: true
 	/// - `with_film_strip`: true
 	pub fn new() -> Self {
-		Default::default()
+		Self::default()
 	}
 
 	/// To respect or not the aspect ratio from the video file in the generated thumbnail
-	pub fn maintain_aspect_ratio(mut self, maintain_aspect_ratio: bool) -> Self {
+	pub const fn maintain_aspect_ratio(mut self, maintain_aspect_ratio: bool) -> Self {
 		self.maintain_aspect_ratio = maintain_aspect_ratio;
 		self
 	}
 
 	/// To set a thumbnail size, respecting or not its aspect ratio, according to `maintain_aspect_ratio` value
-	pub fn size(mut self, size: u32) -> Self {
+	pub const fn size(mut self, size: u32) -> Self {
 		self.size = ThumbnailSize::Size(size);
 		self
 	}
 
 	/// To specify width and height of the thumbnail
-	pub fn width_and_height(mut self, width: u32, height: u32) -> Self {
+	pub const fn width_and_height(mut self, width: u32, height: u32) -> Self {
 		self.size = ThumbnailSize::Dimensions { width, height };
 		self
 	}
@@ -167,19 +168,20 @@ impl ThumbnailerBuilder {
 
 	/// To use embedded metadata in the video file, if available, instead of getting a frame as a
 	/// thumbnail
-	pub fn prefer_embedded_metadata(mut self, prefer_embedded_metadata: bool) -> Self {
+	pub const fn prefer_embedded_metadata(mut self, prefer_embedded_metadata: bool) -> Self {
 		self.prefer_embedded_metadata = prefer_embedded_metadata;
 		self
 	}
 
 	/// If `with_film_strip` is true, a film strip will be added to the thumbnail borders
-	pub fn with_film_strip(mut self, with_film_strip: bool) -> Self {
+	pub const fn with_film_strip(mut self, with_film_strip: bool) -> Self {
 		self.with_film_strip = with_film_strip;
 		self
 	}
 
 	/// Builds a `Thumbnailer` struct
-	pub fn build(self) -> Thumbnailer {
+	#[must_use]
+	pub const fn build(self) -> Thumbnailer {
 		Thumbnailer { builder: self }
 	}
 }

crates/ffmpeg/src/utils.rs

@@ -2,7 +2,7 @@ use crate::error::ThumbnailerError;
 use std::ffi::CString;
 use std::path::Path;
 
-pub(crate) fn from_path(path: impl AsRef<Path>) -> Result<CString, ThumbnailerError> {
+pub fn from_path(path: impl AsRef<Path>) -> Result<CString, ThumbnailerError> {
 	let path = path.as_ref();
 	let path_str = path.as_os_str();
 

crates/ffmpeg/src/video_frame.rs

@@ -2,13 +2,13 @@ use crate::error::FfmpegError;
 use ffmpeg_sys_next::{av_frame_alloc, av_frame_free, AVFrame};
 
 #[derive(Debug)]
-pub(crate) enum FrameSource {
+pub enum FrameSource {
 	VideoStream,
 	Metadata,
 }
 
 #[derive(Debug, Default)]
-pub(crate) struct VideoFrame {
+pub struct VideoFrame {
 	pub width: u32,
 	pub height: u32,
 	pub line_size: u32,
@@ -16,12 +16,12 @@ pub(crate) struct VideoFrame {
 	pub source: Option<FrameSource>,
 }
 
-pub(crate) struct FfmpegFrame {
+pub struct FfmpegFrame {
 	data: *mut AVFrame,
 }
 
 impl FfmpegFrame {
-	pub(crate) fn new() -> Result<Self, FfmpegError> {
+	pub fn new() -> Result<Self, FfmpegError> {
 		let data = unsafe { av_frame_alloc() };
 		if data.is_null() {
 			return Err(FfmpegError::FrameAllocation);
@@ -29,7 +29,7 @@ impl FfmpegFrame {
 		Ok(Self { data })
 	}
 
-	pub(crate) fn as_mut_ptr(&mut self) -> *mut AVFrame {
+	pub fn as_mut_ptr(&mut self) -> *mut AVFrame {
 		self.data
 	}
 }