Skip to content

Commit

Permalink
Fix ffmpeg rotation filter breaking portrait video thumbnails #2150
Browse files Browse the repository at this point in the history 
 - Plus some other misc fixes
  • Loading branch information
@HeavenVolkoff
HeavenVolkoff committed May 1, 2024
1 parent dd574c9 commit 7b3d6b7
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 188 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion core/src/location/non_indexed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ pub async fn walk(
date_modified: entry.metadata.modified_or_now().into(),
size_in_bytes_bytes: entry.metadata.len().to_be_bytes().to_vec(),
},
has_created_thumbnail: false,
has_created_thumbnail: true,
}))
.await?;
}
Expand Down
13 changes: 9 additions & 4 deletions core/src/object/media/old_thumbnail/process.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,9 +470,14 @@ async fn generate_video_thumbnail(
file_path: impl AsRef<Path>,
output_path: impl AsRef<Path>,
) -> Result<(), ThumbnailerError> {
use sd_ffmpeg::to_thumbnail;
use sd_ffmpeg::{to_thumbnail, ThumbnailSize};

to_thumbnail(file_path, output_path, 256, TARGET_QUALITY)
.await
.map_err(Into::into)
to_thumbnail(
file_path,
output_path,
ThumbnailSize::Scale(256),
TARGET_QUALITY,
)
.await
.map_err(Into::into)
}
4 changes: 3 additions & 1 deletion crates/ffmpeg/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ edition = { workspace = true }

[dependencies]
chrono = { workspace = true }
ffmpeg-sys-next = "6.0.1"
image = { workspace = true }
libc = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true, features = ["fs", "rt"] }
tracing = { workspace = true }
webp = { workspace = true }

ffmpeg-sys-next = "6.0.1"

[dev-dependencies]
tempfile = { workspace = true }
tokio = { workspace = true, features = ["fs", "rt", "macros"] }
47 changes: 2 additions & 45 deletions crates/ffmpeg/src/filter_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use ffmpeg_sys_next::{
avfilter_graph_create_filter, avfilter_graph_free, avfilter_link, AVFilterContext,
AVFilterGraph, AVRational,
};

pub(crate) struct FFmpegFilterGraph(*mut AVFilterGraph);

impl<'a> FFmpegFilterGraph {
Expand All @@ -39,7 +38,6 @@ impl<'a> FFmpegFilterGraph {
size: Option<ThumbnailSize>,
time_base: &AVRational,
codec_ctx: &FFmpegCodecContext,
rotation_angle: f64,
interlaced_frame: bool,
pixel_aspect_ratio: &AVRational,
maintain_aspect_ratio: bool,
Expand Down Expand Up @@ -115,55 +113,14 @@ impl<'a> FFmpegFilterGraph {
"Failed to create format filter",
)?;

let mut rotate_filter = ptr::null_mut();
if rotation_angle < -135.0 {
filter_graph.setup_filter(
&mut rotate_filter,
c"rotate",
c"thumb_rotate",
Some(c"PI"),
"Failed to create rotate filter",
)?;
} else if rotation_angle > 45.0 && rotation_angle < 135.0 {
filter_graph.setup_filter(
&mut rotate_filter,
c"transpose",
c"thumb_transpose",
Some(c"2"),
"Failed to create transpose filter",
)?;
} else if rotation_angle < -45.0 && rotation_angle > -135.0 {
filter_graph.setup_filter(
&mut rotate_filter,
c"transpose",
c"thumb_transpose",
Some(c"1"),
"Failed to create transpose filter",
)?;
}

Self::link(
if rotate_filter.is_null() {
format_filter
} else {
rotate_filter
},
format_filter,
0,
filter_sink_ctx,
0,
"Failed to link final filter",
)?;

if !rotate_filter.is_null() {
Self::link(
format_filter,
0,
rotate_filter,
0,
"Failed to link format filter",
)?;
}

Self::link(
scale_filter,
0,
Expand Down Expand Up @@ -253,7 +210,7 @@ fn thumb_scale_filter_args(
) -> Result<String, Error> {
let (width, height) = match size {
Some(ThumbnailSize::Dimensions { width, height }) => (width, Some(height)),
Some(ThumbnailSize::Size(width)) => (width, None),
Some(ThumbnailSize::Scale(width)) => (width, None),
None => return Ok("w=0:h=0".to_string()),
};

Expand Down
113 changes: 51 additions & 62 deletions crates/ffmpeg/src/format_ctx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use crate::{

use ffmpeg_sys_next::{
av_cmp_q, av_display_rotation_get, av_read_frame, av_reduce, av_stream_get_side_data,
avformat_close_input, avformat_find_stream_info, avformat_open_input, AVCodecID, AVDictionary,
AVFormatContext, AVMediaType, AVPacket, AVPacketSideDataType, AVRational, AVStream,
AV_DISPOSITION_ATTACHED_PIC, AV_DISPOSITION_CAPTIONS, AV_DISPOSITION_CLEAN_EFFECTS,
avformat_close_input, avformat_find_stream_info, avformat_open_input, AVChapter, AVCodecID,
AVDictionary, AVFormatContext, AVMediaType, AVPacket, AVPacketSideDataType, AVRational,
AVStream, AV_DISPOSITION_ATTACHED_PIC, AV_DISPOSITION_CAPTIONS, AV_DISPOSITION_CLEAN_EFFECTS,
AV_DISPOSITION_COMMENT, AV_DISPOSITION_DEFAULT, AV_DISPOSITION_DEPENDENT,
AV_DISPOSITION_DESCRIPTIONS, AV_DISPOSITION_DUB, AV_DISPOSITION_FORCED,
AV_DISPOSITION_HEARING_IMPAIRED, AV_DISPOSITION_KARAOKE, AV_DISPOSITION_LYRICS,
Expand Down Expand Up @@ -116,22 +116,22 @@ impl FFmpegFormatContext {
}
}

pub(crate) fn read_frame(&mut self, packet: *mut AVPacket) -> Result<(), Error> {
pub(crate) fn read_frame(&mut self, packet: *mut AVPacket) -> Result<&mut Self, Error> {
check_error(
unsafe { av_read_frame(self.as_mut(), packet) },
"Fail to read the next frame of a media file",
)?;

Ok(())
Ok(self)
}

pub(crate) fn find_stream_info(&mut self) -> Result<(), Error> {
pub(crate) fn find_stream_info(&mut self) -> Result<&mut Self, Error> {
check_error(
unsafe { avformat_find_stream_info(self.as_mut(), ptr::null_mut()) },
"Fail to read packets of a media file to get stream information",
)?;

Ok(())
Ok(self)
}

pub(crate) fn find_preferred_video_stream(
Expand Down Expand Up @@ -232,30 +232,12 @@ impl FFmpegFormatContext {

fn chapters(&self) -> Vec<MediaChapter> {
let chapters_ptr = self.as_ref().chapters;

let Ok(num_chapters) = isize::try_from(self.as_ref().nb_chapters) else {
return vec![];
};

(!chapters_ptr.is_null())
.then(|| {
(0..num_chapters)
.filter_map(|id| {
unsafe { (*(chapters_ptr.offset(id))).as_ref() }.map(|chapter| {
MediaChapter {
// Note: id is guaranteed to be a valid u32 because it was calculated from a u32
id: id as u32,
start: chapter.start,
end: chapter.end,
time_base_num: chapter.time_base.num,
time_base_den: chapter.time_base.den,
metadata: unsafe { chapter.metadata.as_mut() }
.map(|metadata| FFmpegDict::new(Some(metadata)).into())
.unwrap_or_else(MediaMetadata::default),
}
})
})
.collect::<Vec<MediaChapter>>()
(0..isize::try_from(self.as_ref().nb_chapters).unwrap_or(0))
.filter_map(|id| unsafe { (*(chapters_ptr.offset(id))).as_ref() })
.map(|chapter| chapter.into())
.collect()
})
.unwrap_or(vec![])
}
Expand All @@ -266,37 +248,29 @@ impl FFmpegFormatContext {

let mut programs = (!programs_ptr.is_null())
.then(|| {
let Ok(num_programs) = isize::try_from(self.as_ref().nb_programs) else {
return vec![];
};

(0..num_programs)
.filter_map(|id| {
unsafe { (*(programs_ptr.offset(id))).as_ref() }.map(|program| {
let (metadata, name) =
extract_name_and_convert_metadata(program.metadata);

let streams = (0..num_programs)
.filter_map(|index| {
unsafe { program.stream_index.offset(index).as_ref() }.and_then(
|stream_index| {
self.stream(*stream_index).map(|stream| {
visited_streams.insert(*stream_index);
(&*stream).into()
})
},
)
})
.collect::<Vec<MediaStream>>();

MediaProgram {
// Note: id is guaranteed to be a valid u32 because it was calculated from a u32
id: id as u32,
name,
streams,
metadata,
}
})
(0..isize::try_from(self.as_ref().nb_programs).unwrap_or(0))
.filter_map(|id| unsafe { (*(programs_ptr.offset(id))).as_ref() })
.map(|program| {
let (metadata, name) = extract_name_and_convert_metadata(program.metadata);

let streams = (0..isize::try_from(program.nb_stream_indexes).unwrap_or(0))
.filter_map(|index| unsafe {
program.stream_index.offset(index).as_ref()
})
.copied()
.filter_map(|stream_index| {
visited_streams.insert(stream_index);
self.stream(stream_index)
})
.map(|stream| (&*stream).into())
.collect::<Vec<MediaStream>>();

MediaProgram {
id: program.id.unsigned_abs(),
name,
streams,
metadata,
}
})
.collect::<Vec<MediaProgram>>()
})
Expand All @@ -322,8 +296,8 @@ impl FFmpegFormatContext {
}

fn metadata(&self) -> Option<MediaMetadata> {
unsafe { (*(self.0)).metadata.as_mut() }
.map(|metadata| FFmpegDict::new(Some(metadata)).into())
let fmt_ctx = self.as_ref();
unsafe { fmt_ctx.metadata.as_mut() }.map(|metadata| FFmpegDict::new(Some(metadata)).into())
}
}

Expand All @@ -350,6 +324,21 @@ impl From<&FFmpegFormatContext> for MediaInfo {
}
}

impl From<&AVChapter> for MediaChapter {
fn from(chapter: &AVChapter) -> Self {
MediaChapter {
id: chapter.id.unsigned_abs(),
start: chapter.start,
end: chapter.end,
time_base_num: chapter.time_base.num,
time_base_den: chapter.time_base.den,
metadata: unsafe { chapter.metadata.as_mut() }
.map(|metadata| FFmpegDict::new(Some(metadata)).into())
.unwrap_or_else(MediaMetadata::default),
}
}
}

impl From<&AVStream> for MediaStream {
fn from(stream: &AVStream) -> Self {
let (metadata, name) = extract_name_and_convert_metadata(stream.metadata);
Expand Down
29 changes: 13 additions & 16 deletions crates/ffmpeg/src/frame_decoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::{
error::{Error, FFmpegError},
filter_graph::FFmpegFilterGraph,
format_ctx::FFmpegFormatContext,
probe::probe,
utils::{check_error, from_path},
video_frame::FFmpegFrame,
};
Expand All @@ -20,15 +19,16 @@ use ffmpeg_sys_next::{

#[derive(Debug, Clone, Copy)]
pub enum ThumbnailSize {
Scale(u32),
Dimensions { width: u32, height: u32 },
Size(u32),
}

#[derive(Debug)]
pub(crate) struct VideoFrame {
pub data: Vec<u8>,
pub width: u32,
pub height: u32,
pub rotation: f64,
}

pub struct FrameDecoder {
Expand All @@ -49,9 +49,6 @@ impl FrameDecoder {
) -> Result<Self, Error> {
let filename = filename.as_ref();

// TODO: Remove this, just here to test and so clippy stops complaining about it being unused
let _ = probe(filename);

let mut format_context = FFmpegFormatContext::open_file(from_path(filename)?.as_c_str())?;

format_context.find_stream_info()?;
Expand Down Expand Up @@ -163,16 +160,10 @@ impl FrameDecoder {
av_guess_sample_aspect_ratio(self.format_ctx.as_mut(), stream_ptr, self.frame.as_mut())
};

let rotation_angle = self
.format_ctx
.get_stream_rotation_angle(self.preferred_stream_id)
.round();

let (_guard, filter_source, filter_sink) = FFmpegFilterGraph::thumbnail_graph(
size,
&time_base,
&self.codec_ctx,
rotation_angle,
(self.frame.as_mut().flags & AV_FRAME_FLAG_INTERLACED) != 0,
&pixel_aspect_ratio,
maintain_aspect_ratio,
Expand All @@ -195,17 +186,23 @@ impl FrameDecoder {
}
check_error(get_frame_errno, "Failed to get buffer from filter")?;

let height = new_frame.as_ref().height;
let line_size = new_frame.as_ref().linesize[0];
let mut data = Vec::with_capacity(usize::try_from(line_size * height)?);
let width = new_frame.as_ref().width.unsigned_abs();
let height = new_frame.as_ref().height.unsigned_abs();
let line_size = usize::try_from(new_frame.as_ref().linesize[0])?;

let mut data = Vec::with_capacity(line_size * usize::try_from(height)?);
data.extend_from_slice(unsafe {
std::slice::from_raw_parts(new_frame.as_ref().data[0], data.capacity())
});

Ok(VideoFrame {
height: u32::try_from(height)?,
width: new_frame.as_ref().width.try_into()?,
data,
width,
height,
rotation: self
.format_ctx
.get_stream_rotation_angle(self.preferred_stream_id)
.round(),
})
}

Expand Down
Loading

0 comments on commit 7b3d6b7

Please sign in to comment.