From c03b0a7aa710c81627cd9ffaf62811976b7d9c76 Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Sun, 11 Oct 2026 14:30:44 +0200 Subject: [PATCH] Import: MP4/MOV movies respect their rotation A movie shot with a phone held upright stores sideways frames and says so in its video track header; it imported lying on its side. Probing now reports the turned size, and decoded frames (full and reduced size) are turned to match. Only the rotation is used, snapped to a quarter turn; mirroring, scaling and offsets in the header are ignored. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + crates/media/Cargo.toml | 1 + crates/media/src/convert.rs | 26 +++++++++++++++++ crates/media/src/pool.rs | 29 ++++++++++++------ crates/media/src/probe.rs | 32 ++++++++++++++++++-- crates/media/tests/footage.rs | 55 ++++++++++++++++++++++++++++++++++- 6 files changed, 132 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ba84c75f..5dc65a2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1430,6 +1430,7 @@ dependencies = [ "filmcraft-codecs", "filmcraft-color", "filmcraft-frame", + "filmcraft-isobmff", "filmcraft-media", "filmcraft-time", "gif", diff --git a/crates/media/Cargo.toml b/crates/media/Cargo.toml index 0d7803fd..a5056c04 100644 --- a/crates/media/Cargo.toml +++ b/crates/media/Cargo.toml @@ -20,6 +20,7 @@ effectcraft-pdf = { workspace = true } effectcraft-model = { workspace = true } filmcraft-codecs = { workspace = true } filmcraft-media = { workspace = true } +filmcraft-isobmff = { workspace = true } filmcraft-frame = { workspace = true } filmcraft-color = { workspace = true } filmcraft-time = { workspace = true } diff --git a/crates/media/src/convert.rs b/crates/media/src/convert.rs index 7865c3dc..6ca9c9f9 100644 --- a/crates/media/src/convert.rs +++ b/crates/media/src/convert.rs @@ -104,6 +104,32 @@ pub(crate) fn frame_to_image_in(f: &VideoFrame, op: AlphaOp, mut buf: Vec) - img } +/// A movie frame turned the way its file says it is shown ([`crate::probe::video_turn`]: +/// clockwise quarter turns; anything else leaves it as decoded). +pub(crate) fn turned(img: Image, turn: image::metadata::Orientation) -> Image { + use image::metadata::Orientation; + let (w, h) = (img.width as usize, img.height as usize); + if img.data.len() != w * h || w == 0 || h == 0 { + return img; + } + // The source pixel shown at (x, y) of the turned picture. + let (tw, th, from): (usize, usize, fn(usize, usize, usize, usize) -> usize) = match turn { + Orientation::Rotate90 => (h, w, |x, y, w, h| (h - 1 - x) * w + y), + Orientation::Rotate180 => (w, h, |x, y, w, h| (h - 1 - y) * w + (w - 1 - x)), + Orientation::Rotate270 => (h, w, |x, y, w, _| x * w + (w - 1 - y)), + _ => return img, + }; + let mut data = vec![[0.0; 4]; tw * th]; + data.par_chunks_mut(tw).enumerate().for_each(|(y, row)| { + for (x, px) in row.iter_mut().enumerate() { + if let Some(s) = img.data.get(from(x, y, w, h)) { + *px = *s; + } + } + }); + Image { width: tw as u32, height: th as u32, data } +} + /// A FilmCraft frame for a render that shows it at `w` × `h`, at most half its size each way: /// what `resample(&frame_to_image(f, op), w, h)` makes, converting a quarter of the pixels. /// Opaque 4:2:0 Y'CbCr is box-filtered to half size before conversion (each 2×2 block of luma, diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index cdec5e3f..ce4be9c2 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -9,8 +9,9 @@ use effectcraft_raster::{Image, Px}; use effectcraft_render::FootageSource; use effectcraft_time::{FrameRate, TICKS_PER_SECOND, Tick}; use filmcraft_media::{FrameRequest, SharedSource}; +use image::metadata::Orientation; -use crate::convert::{AlphaOp, dynamic_to_image, frame_to_image_in, frame_to_image_scaled}; +use crate::convert::{AlphaOp, dynamic_to_image, frame_to_image_in, frame_to_image_scaled, turned}; use crate::gif_anim::{self, Anim}; use crate::{MediaError, Result}; @@ -170,9 +171,10 @@ struct CacheState { last: HashMap, i64>, } -/// A movie/audio source by path, opened by the first caller while the others wait for it +/// A movie/audio source by path and the turn its picture is shown with +/// ([`crate::probe::video_turn`]), opened by the first caller while the others wait for it /// (`None`: failed to open; not retried until `forget`). -type SourceSlot = Arc>>; +type SourceSlot = Arc>>; /// An animated GIF by path, as [`SourceSlot`] (the error message when it failed to load). type GifSlot = Arc, String>>>; @@ -572,12 +574,18 @@ impl Inner { /// it again: a preview's frame workers, audio feeder, conform and waveform threads each /// decoded a long FLAC in full at once (#408). fn source(&self, path: &Arc) -> Option { + self.opened(path).map(|(src, _)| src) + } + + /// [`Self::source`] and the turn the movie's picture is shown with. + fn opened(&self, path: &Arc) -> Option<(SharedSource, Orientation)> { let slot = lock(&self.sources).entry(path.clone()).or_default().clone(); slot.get_or_init(|| { self.opened.fetch_add(1, Ordering::Relaxed); let opened = self.read(path).and_then(|bytes| { let name = std::path::Path::new(&**path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(); - filmcraft_codecs::open_bytes(&name, bytes).map_err(MediaError::from) + let turn = crate::probe::video_turn(&bytes); + filmcraft_codecs::open_bytes(&name, bytes).map(|src| (src, turn)).map_err(MediaError::from) }); match opened { Ok(s) => Some(s), @@ -751,19 +759,22 @@ impl Inner { Ok(dynamic_to_image(frame, op)) } Some(mt) => { - let src = self.source(path).ok_or_else(|| MediaError::Io(format!("{path}: cannot open")))?; + let (src, turn) = self.opened(path).ok_or_else(|| MediaError::Io(format!("{path}: cannot open")))?; let vf = src.video_frame(FrameRequest::full(filmcraft_time::Tick(mt.0))).map_err(MediaError::from)?; + // A movie shown turned a quarter (a phone held upright): the footage's size and + // the render's are the turned picture's, the decoded frame's are the file's. + let side = |(w, h): (u32, u32)| if crate::probe::swaps_sides(turn) { (h, w) } else { (w, h) }; // A reduced-resolution render's frame, made at its size (a frame of the footage's // own size only, as the renderer resamples only those). - let (w, h) = loc.key.size; + let (w, h) = side(loc.key.size); if w > 0 - && (vf.width, vf.height) == (footage.width, footage.height) + && (vf.width, vf.height) == side((footage.width, footage.height)) && let Some(img) = frame_to_image_scaled(&vf, w, h) { - return Ok(img); + return Ok(turned(img, turn)); } let buf = lock(&self.cache).lru.take_spare(vf.width as usize * vf.height as usize); - Ok(frame_to_image_in(&vf, op, buf)) + Ok(turned(frame_to_image_in(&vf, op, buf), turn)) } } } diff --git a/crates/media/src/probe.rs b/crates/media/src/probe.rs index d443edce..aa9db2ff 100644 --- a/crates/media/src/probe.rs +++ b/crates/media/src/probe.rs @@ -75,8 +75,17 @@ pub fn probe_bytes(path: &str, bytes: Arc<[u8]>) -> Result { return probe_still_bytes(path, &bytes, fmt); } let name = Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()); + let turn = video_turn(&bytes); let src = filmcraft_codecs::open_bytes(&name, bytes)?; - Ok(footage_from_info(path, src.info())) + let mut f = footage_from_info(path, src.info()); + // The size as displayed: a movie shot with the camera on its side trades width and height. + if swaps_sides(turn) { + std::mem::swap(&mut f.width, &mut f.height); + if f.pixel_aspect > 0.0 { + f.pixel_aspect = 1.0 / f.pixel_aspect; + } + } + Ok(f) } pub(crate) fn footage_from_info(path: &str, info: &filmcraft_media::MediaInfo) -> Footage { @@ -183,7 +192,7 @@ fn probe_still_bytes(path: &str, bytes: &[u8], fmt: image::ImageFormat) -> Resul } /// Whether an Exif orientation turns the picture a quarter turn (its width and height trade places). -fn swaps_sides(o: Orientation) -> bool { +pub(crate) fn swaps_sides(o: Orientation) -> bool { matches!(o, Orientation::Rotate90 | Orientation::Rotate270 | Orientation::Rotate90FlipH | Orientation::Rotate270FlipH) } @@ -198,3 +207,22 @@ pub(crate) fn load_oriented(bytes: &[u8]) -> image::ImageResult Orientation { + let Ok(file) = filmcraft_isobmff::open(bytes) else { return Orientation::NoTransforms }; + let Some(m) = file.tracks.iter().find(|t| t.kind == filmcraft_isobmff::TrackKind::Video).map(|t| t.matrix) else { + return Orientation::NoTransforms; + }; + // [a b u; c d v; x y w]: (a, b) is where the picture's x axis points. + let (a, b) = (m[0] as i64, m[1] as i64); + match (a.abs() >= b.abs(), a >= 0, b >= 0) { + (true, true, _) => Orientation::NoTransforms, + (true, false, _) => Orientation::Rotate180, + (false, _, true) => Orientation::Rotate90, + (false, _, false) => Orientation::Rotate270, + } +} diff --git a/crates/media/tests/footage.rs b/crates/media/tests/footage.rs index d26b1a1d..8abc77b2 100644 --- a/crates/media/tests/footage.rs +++ b/crates/media/tests/footage.rs @@ -9,7 +9,7 @@ use std::process::Command; use std::sync::Arc; use std::time::Instant; -use effectcraft_media::{MediaPool, probe}; +use effectcraft_media::{MediaPool, probe, probe_bytes}; use effectcraft_project::{AlphaMode, Footage, FootageKind, ItemId}; use effectcraft_raster::Image; use effectcraft_render::FootageSource; @@ -336,6 +336,59 @@ fn resampled_audio_read_in_frame_pieces_matches_ffmpeg() { } } +/// The H.264 fixture with the rotation part of its video track header's matrix (`[a b; c d]`, +/// 16.16 fixed point) replaced: what a phone writes for a movie shot with the camera turned. +fn turned_mp4(p: &Path, abcd: [i32; 4]) -> Arc<[u8]> { + let mut bytes = std::fs::read(p).expect("read"); + // (The video track comes first; a version 0 `tkhd` has its matrix 44 bytes after the type.) + let at = bytes.windows(4).position(|w| w == b"tkhd").expect("tkhd"); + assert_eq!(bytes[at + 4], 0, "a version 0 tkhd"); + for (slot, v) in [0, 1, 3, 4].into_iter().zip(abcd) { + let o = at + 44 + slot * 4; + bytes[o..o + 4].copy_from_slice(&v.to_be_bytes()); + } + bytes.into() +} + +/// A movie whose track header says it is shown rotated (a phone held upright) imports the way it +/// is shown: the footage has the turned size and its frames are turned, at full and reduced size. +#[test] +fn rotated_mp4_imports_upright() { + let Some(p) = h264_mp4() else { return }; + let plain_footage = probe(&p).expect("probe"); + let pool = MediaPool::new(); + let t = t_of(plain_footage.frame_rate, 3); + let plain = pool.frame(ItemId(1), &plain_footage, t).expect("frame"); + let (w, h) = (plain.width as usize, plain.height as usize); + let one = 0x1_0000; + // The matrix, the turned size and the plain frame's pixel shown at (x, y). + type From = fn(usize, usize, usize, usize) -> (usize, usize); + let cases: [(&str, [i32; 4], (usize, usize), From); 3] = [ + ("90", [0, one, -one, 0], (h, w), |x, y, _, h| (y, h - 1 - x)), + ("180", [-one, 0, 0, -one], (w, h), |x, y, w, h| (w - 1 - x, h - 1 - y)), + ("270", [0, -one, one, 0], (h, w), |x, y, w, _| (w - 1 - y, x)), + ]; + for (name, matrix, (tw, th), from) in cases { + let path = format!("turned-{name}.mp4"); + let bytes = turned_mp4(&p, matrix); + let f = probe_bytes(&path, bytes.clone()).expect("probe"); + assert_eq!((f.width as usize, f.height as usize), (tw, th), "{name}°: size"); + pool.add_bytes(&path, bytes); + let img = pool.frame(ItemId(2), &f, t).expect("frame"); + assert_eq!((img.width as usize, img.height as usize), (tw, th), "{name}°: frame size"); + for (x, y) in [(0, 0), (tw - 1, 0), (0, th - 1), (tw - 1, th - 1), (tw / 3, th / 5), (tw / 2, th - 7)] { + let (sx, sy) = from(x, y, w, h); + assert_eq!(img.data[y * tw + x], plain.data[sy * w + sx], "{name}°: pixel ({x}, {y})"); + } + // (A reduced-resolution render asks for the turned size.) + let (hw, hh) = (f.width / 2, f.height / 2); + let sized = pool.frame_at_size(ItemId(2), &f, t, hw, hh).expect("frame"); + assert_eq!((sized.width, sized.height), (hw, hh), "{name}°: reduced size"); + let db = psnr(&sized, &effectcraft_raster::resample(&img, hw, hh)); + assert!(db > 45.0, "{name}° at half size: {db:.2} dB"); + } +} + #[test] fn prores_4444_with_alpha() { let Some(p) = prores_mov() else { return };