diff --git a/gifski-api/Cargo.lock b/gifski-api/Cargo.lock index eecdac80..8013a3e4 100644 --- a/gifski-api/Cargo.lock +++ b/gifski-api/Cargo.lock @@ -167,9 +167,9 @@ checksum = "b2641c4a7c0c4101df53ea572bffdc561c146f6c2eb09e4df02bc4811e3feeb4" [[package]] name = "fallible_collections" -version = "0.3.1" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9599e8ccc571becb62700174680e54e5c50fc5b4d34c1c56d8915e0325650fea" +checksum = "74bebf0efe2e883c1619c455e3f1764333064694ebd5125d2faddabfb5963186" dependencies = [ "hashbrown", ] @@ -201,9 +201,9 @@ dependencies = [ [[package]] name = "flate2" -version = "1.0.19" +version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7411863d55df97a419aa64cb4d2f167103ea9d767e2c54a1868b7ac3f6b47129" +checksum = "cd3aec53de10fe96d7d8c565eb17f2c687bb5518a2ec453b5b1252964526abe0" dependencies = [ "cfg-if 1.0.0", "crc32fast", @@ -244,7 +244,7 @@ dependencies = [ [[package]] name = "gifski" -version = "1.3.1" +version = "1.3.3" dependencies = [ "clap", "crossbeam-channel", @@ -301,9 +301,9 @@ dependencies = [ [[package]] name = "imagequant-sys" -version = "3.0.2+sys2.14.0" +version = "3.0.3+sys2.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "718bd7db850c9f66155c8f67e3db7488b1ad3643c0bb88a3e2d17c80a211e0be" +checksum = "3689df1fd74c4a1737ea60ba03a05105d37d0fc8e7ba3028e7e10f162939f8fd" dependencies = [ "bitflags", "cc", @@ -331,9 +331,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.82" +version = "0.2.84" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89203f3fba0a3795506acaad8ebce3c80c0af93f994d5a1d7a0b1eeb23271929" +checksum = "1cca32fa0182e8c0989459524dc356b8f2b5c10f1b9eb521b7d182c03cf8c5ff" [[package]] name = "libloading" @@ -347,9 +347,9 @@ dependencies = [ [[package]] name = "lodepng" -version = "3.4.2" +version = "3.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f637e5831398a7803828d7324e6a185f42aa6857e7766f70476e118d59ad31d1" +checksum = "11443d177d97dc468ee5cc956769bbdeb4c20707d443c62dfd8b473505365613" dependencies = [ "fallible_collections", "flate2", @@ -473,10 +473,11 @@ checksum = "b5eb417147ba9860a96cfe72a0b93bf88fee1744b5636ec99ab20c1aa9376581" [[package]] name = "resize" -version = "0.5.5" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2a08c42ea86684dc00256494c4eb8b54707890ddac50c05060a717f29669029" +checksum = "42ba1357806509e44508dbb41eb60db3e6f6781ef6884f4540b860069adeed9f" dependencies = [ + "fallible_collections", "rgb", ] @@ -565,9 +566,9 @@ checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f" [[package]] name = "weezl" -version = "0.1.3" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e2bb9fc8309084dd7cd651336673844c1d47f8ef6d2091ec160b27f5c4aa277" +checksum = "4a32b378380f4e9869b22f0b5177c68a5519f03b3454fde0b291455ddbae266c" [[package]] name = "wild" diff --git a/gifski-api/Cargo.toml b/gifski-api/Cargo.toml index 057f8697..a6a80346 100644 --- a/gifski-api/Cargo.toml +++ b/gifski-api/Cargo.toml @@ -10,7 +10,7 @@ license = "AGPL-3.0+" name = "gifski" readme = "README.md" repository = "https://github.com/ImageOptim/gifski" -version = "1.3.1" +version = "1.3.3" autobins = false edition = "2018" @@ -23,11 +23,11 @@ gifsicle = { version = "1.92.4", optional = true } clap = "2.33.3" gif = "0.11.1" gif-dispose = "3.1.1" -imagequant = "3.0.2" +imagequant = "3.0.3" imgref = "1.7.1" -lodepng = "3.4.2" +lodepng = "3.4.3" pbr = "1.0.4" -resize = "0.5.5" +resize = "0.6.1" rgb = "0.8.25" wild = "2.0.4" natord = "1.0.9" @@ -53,8 +53,13 @@ video-static = ["video", "ffmpeg/build"] path = "src/lib.rs" crate-type = ["lib", "staticlib", "cdylib"] -[profile.dev.package.imagequant] -opt-level = 3 +[profile.dev] +debug = 1 +opt-level = 1 + +[profile.dev.package.'*'] +opt-level = 2 +debug = false [profile.release] panic = "abort" diff --git a/gifski-api/README.md b/gifski-api/README.md index 2a306352..527fe4af 100644 --- a/gifski-api/README.md +++ b/gifski-api/README.md @@ -34,6 +34,8 @@ gifski -o anim.gif frame*.png You can also resize frames (with `-W ` option). If the input was ever encoded using a lossy video codec it's recommended to at least halve size of the frames to hide compression artefacts and counter chroma subsampling that was done by the video codec. +Adding `--quality=90` may reduce file sizes a bit, but expect to lose a lot of quality for little gain. GIF just isn't that good at compressing, no matter how much you compromise. + See `gifski -h` for more options. ## Building diff --git a/gifski-api/src/bin/ffmpeg_source.rs b/gifski-api/src/bin/ffmpeg_source.rs index f8d400f8..fd688609 100644 --- a/gifski-api/src/bin/ffmpeg_source.rs +++ b/gifski-api/src/bin/ffmpeg_source.rs @@ -1,9 +1,9 @@ +use crate::source::*; use crate::BinResult; use gifski::Collector; use gifski::Settings; use imgref::*; use rgb::*; -use crate::source::*; use std::path::Path; pub struct FfmpegDecoder { @@ -89,12 +89,12 @@ impl FfmpegDecoder { let mut filt_frame = ffmpeg::util::frame::Video::empty(); let mut i = 0; let mut pts_last_packet = 0; - let mut delayed_frames = 0; let pts_frame_step = 1.0 / self.rate.fps as f64; loop { - let (packet, packet_is_empty) = if let Some((s, packet)) = packets.next() { + let (packet, no_more_packets) = if let Some((s, packet)) = packets.next() { if s.index() != stream_index { + // ignore irrelevant streams continue; } pts_last_packet = packet.pts().ok_or("ffmpeg format error")? + packet.duration(); @@ -102,30 +102,24 @@ impl FfmpegDecoder { } else { (ffmpeg::Packet::empty(), true) }; + let decoded = decoder.decode(&packet, &mut vid_frame)?; - if !decoded || 0 == vid_frame.width() { - if packet_is_empty { - if delayed_frames == 0 { - break; - } - } else { - delayed_frames += 1; + if decoded { + filter.get("in").ok_or("ffmpeg format error")?.source().add(&vid_frame)?; + let mut out = filter.get("out").ok_or("ffmpeg format error")?; + let mut out = out.sink(); + while let Ok(..) = out.frame(&mut filt_frame) { + add_frame(&filt_frame, pts_frame_step * i as f64, i)?; + i += 1; } - continue; } - if packet_is_empty { - delayed_frames -= 1; - } - - filter.get("in").ok_or("ffmpeg format error")?.source().add(&vid_frame)?; - let mut out = filter.get("out").ok_or("ffmpeg format error")?; - let mut out = out.sink(); - while let Ok(..) = out.frame(&mut filt_frame) { - add_frame(&filt_frame, pts_frame_step * i as f64, i)?; - i += 1; + // loop to flush decoder's buffer + if no_more_packets && !decoded { + break; } } + // now flush filter's buffer filter.get("in").ok_or("ffmpeg format error")?.source().close(pts_last_packet)?; let mut out = filter.get("out").ok_or("ffmpeg format error")?; let mut out = out.sink(); diff --git a/gifski-api/src/bin/gifski.rs b/gifski-api/src/bin/gifski.rs index 423e2a47..233b9772 100644 --- a/gifski-api/src/bin/gifski.rs +++ b/gifski-api/src/bin/gifski.rs @@ -2,8 +2,6 @@ use std::ffi::OsStr; use gifski::{Settings, Repeat}; -use natord; -use wild; #[cfg(feature = "video")] mod ffmpeg_source; @@ -19,16 +17,16 @@ use clap::{App, AppSettings, Arg}; use std::env; use std::fmt; -use std::io; use std::fs::File; +use std::io; use std::path::{Path, PathBuf}; use std::thread; use std::time::Duration; #[cfg(feature = "video")] -const VIDEO_FRAMES_ARG_HELP: &'static str = "one video file supported by FFmpeg, or multiple PNG image files"; +const VIDEO_FRAMES_ARG_HELP: &str = "one video file supported by FFmpeg, or multiple PNG image files"; #[cfg(not(feature = "video"))] -const VIDEO_FRAMES_ARG_HELP: &'static str = "PNG image files"; +const VIDEO_FRAMES_ARG_HELP: &str = "PNG image files"; fn main() { if let Err(e) = bin_main() { @@ -40,8 +38,9 @@ fn main() { } } +#[allow(clippy::float_cmp)] fn bin_main() -> BinResult<()> { - let matches = App::new(crate_name!()) + let matches = App::new(crate_name!()) .version(crate_version!()) .about("https://gif.ski by Kornel LesiƄski") .setting(AppSettings::UnifiedHelpMessage) @@ -71,7 +70,7 @@ fn bin_main() -> BinResult<()> { .long("fast-forward") .help("Multiply speed of video by a factor\n(no effect when using PNG files as input)") .empty_values(false) - .value_name("num") + .value_name("x") .default_value("1")) .arg(Arg::with_name("fast") .long("fast") @@ -118,7 +117,7 @@ fn bin_main() -> BinResult<()> { if !matches.is_present("nosort") { frames.sort_by(|a, b| natord::compare(a, b)); } - let frames: Vec<_> = frames.into_iter().map(|s| PathBuf::from(s)).collect(); + let frames: Vec<_> = frames.into_iter().map(PathBuf::from).collect(); let output_path = DestPath::new(matches.value_of_os("output").ok_or("Missing output")?); let width = parse_opt(matches.value_of("width")).map_err(|_| "Invalid width")?; @@ -142,10 +141,7 @@ fn bin_main() -> BinResult<()> { let fps: f32 = matches.value_of("fps").ok_or("Missing fps")?.parse().map_err(|_| "FPS must be a number")?; let speed: f32 = matches.value_of("fast-forward").ok_or("Missing speed")?.parse().map_err(|_| "Speed must be a number")?; - let rate = source::Fps { - speed, - fps, - }; + let rate = source::Fps { speed, fps }; if settings.quality < 20 { if settings.quality < 1 { @@ -265,6 +261,7 @@ fn get_video_decoder(path: &Path, fps: source::Fps, settings: Settings) -> BinRe } #[cfg(not(feature = "video"))] +#[cold] fn get_video_decoder(_: &Path, _: source::Fps, _: Settings) -> BinResult> { Err(r"Video support is permanently disabled in this executable. diff --git a/gifski-api/src/bin/png.rs b/gifski-api/src/bin/png.rs index 1333564d..efbf6ff9 100644 --- a/gifski-api/src/bin/png.rs +++ b/gifski-api/src/bin/png.rs @@ -1,5 +1,5 @@ -use crate::source::Source; use crate::source::Fps; +use crate::source::Source; use crate::BinResult; use gifski::Collector; use std::path::PathBuf; diff --git a/gifski-api/src/c_api.rs b/gifski-api/src/c_api.rs index 2c65b0f7..0b824952 100644 --- a/gifski-api/src/c_api.rs +++ b/gifski-api/src/c_api.rs @@ -1,3 +1,4 @@ +#![allow(clippy::missing_safety_doc)] //! How to use from C //! //! ```c diff --git a/gifski-api/src/c_api/c_api_error.rs b/gifski-api/src/c_api/c_api_error.rs index cb768109..856cc43b 100644 --- a/gifski-api/src/c_api/c_api_error.rs +++ b/gifski-api/src/c_api/c_api_error.rs @@ -26,6 +26,7 @@ pub enum GifskiError { } impl Into for GifskiError { + #[cold] fn into(self) -> io::Error { use std::io::ErrorKind as EK; use GifskiError::*; @@ -45,6 +46,7 @@ impl Into for GifskiError { } impl From for GifskiError { + #[cold] fn from(res: c_int) -> Self { use GifskiError::*; match res { @@ -69,6 +71,7 @@ impl From for GifskiError { } impl From> for GifskiError { + #[cold] fn from(res: CatResult<()>) -> Self { use crate::error::Error::*; match res { @@ -85,6 +88,7 @@ impl From> for GifskiError { } impl From for GifskiError { + #[cold] fn from(res: io::ErrorKind) -> Self { use std::io::ErrorKind as EK; match res { diff --git a/gifski-api/src/encodegifsicle.rs b/gifski-api/src/encodegifsicle.rs index 932abdce..f66d3db1 100644 --- a/gifski-api/src/encodegifsicle.rs +++ b/gifski-api/src/encodegifsicle.rs @@ -1,7 +1,7 @@ use crate::error::*; -use crate::{Encoder, Repeat}; use crate::GIFFrame; use crate::Settings; +use crate::{Encoder, Repeat}; use gifsicle::*; use std::io::Write; use std::ptr; @@ -86,7 +86,7 @@ impl Encoder for Gifsicle<'_> { unsafe { self.gif_writer = Gif_IncrementalWriteFileInit(gfs, &self.info, ptr::null_mut()); if self.gif_writer.is_null() { - Err(Error::Gifsicle)?; + return Err(Error::Gifsicle); } } } @@ -123,12 +123,12 @@ impl Encoder for Gifsicle<'_> { unsafe { if 0 == Gif_SetUncompressedImage(g, image.buf().as_ptr() as *mut u8, None, 0) { Gif_DeleteImage(g); - Err(Error::Gifsicle)?; + return Err(Error::Gifsicle); } let res = Gif_IncrementalWriteImage(self.gif_writer, self.gfs, g); Gif_DeleteImage(g); if 0 == res { - Err(Error::Gifsicle)?; + return Err(Error::Gifsicle); } self.flush_writer()?; } diff --git a/gifski-api/src/encoderust.rs b/gifski-api/src/encoderust.rs index a1472f47..695037bc 100644 --- a/gifski-api/src/encoderust.rs +++ b/gifski-api/src/encoderust.rs @@ -1,7 +1,7 @@ use crate::error::CatResult; -use crate::{Encoder, Repeat}; use crate::GIFFrame; use crate::Settings; +use crate::{Encoder, Repeat}; use rgb::*; use std::io::Write; diff --git a/gifski-api/src/error.rs b/gifski-api/src/error.rs index c04812f5..b2cce0b5 100644 --- a/gifski-api/src/error.rs +++ b/gifski-api/src/error.rs @@ -1,5 +1,3 @@ -use gif_dispose; -use imagequant; use std::io; quick_error! { @@ -29,6 +27,7 @@ quick_error! { } WrongSize(msg: String) { display("{}", msg) + from(e: resize::Error) -> (e.to_string()) } Quant(liq: imagequant::liq_error) { from() @@ -44,6 +43,7 @@ quick_error! { pub type CatResult = Result; impl From for Error { + #[cold] fn from(err: gif::EncodingError) -> Self { match err { gif::EncodingError::Io(err) => err.into(), @@ -53,12 +53,14 @@ impl From for Error { } impl From> for Error { + #[cold] fn from(_: crossbeam_channel::SendError) -> Self { Self::ThreadSend } } impl From for Error { + #[cold] fn from(_: crossbeam_channel::RecvError) -> Self { Self::Aborted } diff --git a/gifski-api/src/lib.rs b/gifski-api/src/lib.rs index fdfdadf5..5ce30e8d 100644 --- a/gifski-api/src/lib.rs +++ b/gifski-api/src/lib.rs @@ -17,7 +17,8 @@ */ #![doc(html_logo_url = "https://gif.ski/icon.png")] -#[macro_use] extern crate quick_error; +#[macro_use] +extern crate quick_error; use imagequant::*; use imgref::*; @@ -35,9 +36,9 @@ mod encoderust; #[cfg(feature = "gifsicle")] mod encodegifsicle; +use crossbeam_channel::{Receiver, Sender}; use std::io::prelude::*; use std::path::PathBuf; -use crossbeam_channel::{Sender, Receiver}; use std::thread; type DecodedImage = CatResult<(ImgVec, f64)>; @@ -65,20 +66,20 @@ pub struct Settings { } impl Settings { - #[cfg(not(feature = "gifsicle"))] + /// quality is used in other places, like gifsicle or frame differences, + /// and it's better to lower quality there before ruining quantization pub(crate) fn color_quality(&self) -> u8 { - self.quality - } - - #[cfg(feature = "gifsicle")] - pub(crate) fn color_quality(&self) -> u8 { - (self.quality * 2).min(100) + (self.quality as u16 * 4 / 3).min(100) as u8 } /// add_frame is going to resize the images to this size. pub fn dimensions_for_image(&self, width: usize, height: usize) -> (usize, usize) { dimensions_for_image((width, height), (self.width, self.height)) } + + pub(crate) fn gifsicle_loss(&self) -> u32 { + (100./6. - self.quality as f32 / 6.).powf(1.75).ceil() as u32 + } } /// Collect frames that will be encoded @@ -146,7 +147,6 @@ struct FrameMessage { frame: GIFFrame, } - /// Start new encoding /// /// Encoding is multi-threaded, and the `Collector` and `Writer` @@ -179,7 +179,7 @@ impl Collector { /// /// If the first frame doesn't start at pts=0, the delay will be used for the last frame. pub fn add_frame_rgba(&mut self, frame_index: usize, image: ImgVec, presentation_timestamp: f64) -> CatResult<()> { - self.queue.push(frame_index, Ok((Self::resized_binary_alpha(image, self.width, self.height), presentation_timestamp))) + self.queue.push(frame_index, Ok((Self::resized_binary_alpha(image, self.width, self.height)?, presentation_timestamp))) } /// Read and decode a PNG file from disk. @@ -195,19 +195,21 @@ impl Collector { let image = lodepng::decode32_file(&path) .map_err(|err| Error::PNG(format!("Can't load {}: {}", path.display(), err)))?; - self.queue.push(frame_index, Ok((Self::resized_binary_alpha(ImgVec::new(image.buffer, image.width, image.height), width, height), presentation_timestamp))) + self.queue.push(frame_index, Ok((Self::resized_binary_alpha(ImgVec::new(image.buffer, image.width, image.height), width, height)?, presentation_timestamp))) } - fn resized_binary_alpha(mut image: ImgVec, width: Option, height: Option) -> ImgVec { + #[allow(clippy::identity_op)] + #[allow(clippy::erasing_op)] + fn resized_binary_alpha(mut image: ImgVec, width: Option, height: Option) -> CatResult> { let (width, height) = dimensions_for_image((image.width(), image.height()), (width, height)); if width != image.width() || height != image.height() { let (buf, img_width, img_height) = image.into_contiguous_buf(); assert_eq!(buf.len(), img_width * img_height); - let mut r = resize::new(img_width, img_height, width, height, resize::Pixel::RGBA, resize::Type::Lanczos3); + let mut r = resize::new(img_width, img_height, width, height, resize::Pixel::RGBA, resize::Type::Lanczos3)?; let mut dst = vec![RGBA8::new(0, 0, 0, 0); width * height]; - r.resize(buf.as_bytes(), dst.as_bytes_mut()); + r.resize(buf.as_bytes(), dst.as_bytes_mut())?; image = ImgVec::new(dst, width, height) } @@ -229,7 +231,7 @@ impl Collector { } } } - image + Ok(image) } } @@ -238,7 +240,7 @@ impl Collector { fn dimensions_for_image((img_w, img_h): (usize, usize), resize_to: (Option, Option)) -> (usize, usize) { match resize_to { (None, None) => { - let factor = (img_w * img_h + 800*600) / (800*600); + let factor = (img_w * img_h + 800 * 600) / (800 * 600); if factor > 1 { (img_w / factor, img_h / factor) } else { @@ -277,12 +279,12 @@ impl Writer { 100 // the first frame is too important to ruin it }; liq.set_quality(0, quality); - let mut img = liq.new_image_stride_copy(image.buf(), image.width(), image.height(), image.stride(), 0.).expect("stridecopy"); - img.set_importance_map(importance_map).expect("immap"); + let mut img = liq.new_image_stride_copy(image.buf(), image.width(), image.height(), image.stride(), 0.)?; + img.set_importance_map(importance_map)?; if has_prev_frame { img.add_fixed_color(RGBA8::new(0, 0, 0, 0)); } - let res = liq.quantize(&img).expect("quantize"); + let res = liq.quantize(&img)?; Ok((liq, res, img)) } @@ -318,7 +320,7 @@ impl Writer { while n_done < ordinal_frame_number { n_done += 1; if !reporter.increase() { - return Err(Error::Aborted.into()); + return Err(Error::Aborted); } } } @@ -337,8 +339,7 @@ impl Writer { #[cfg(feature = "gifsicle")] { if self.settings.quality < 100 { - let loss = (100 - self.settings.quality as u32) * 6; - let mut gifsicle = encodegifsicle::Gifsicle::new(loss, &mut writer); + let mut gifsicle = encodegifsicle::Gifsicle::new(self.settings.gifsicle_loss(), &mut writer); return self.write_with_encoder(&mut gifsicle, reporter); } } @@ -369,11 +370,12 @@ impl Writer { } fn make_diffs(mut inputs: OrdQueueIter, quant_queue: Sender, _settings: &Settings) -> CatResult<()> { - let mut next_frame = inputs.next().transpose()?; - - let first_frame_pts = next_frame.as_ref().map(|&(_, pts)| pts).unwrap_or_default(); + let (first_frame, first_frame_pts) = inputs.next().transpose()?.ok_or(Error::NoFrames)?; let mut prev_frame_pts = 0.0; + let first_frame_has_transparency = first_frame.pixels().any(|px| px.a < 128); + + let mut next_frame = Some((first_frame, first_frame_pts)); let mut ordinal_frame_number = 0; while let Some((image, mut pts)) = { // this is not while loop's body, but a block that gets the next element @@ -409,7 +411,12 @@ impl Writer { importance_map } else { // Last frame should reset to background to avoid breaking transparent looped anims - dispose = gif::DisposalMethod::Background; + if first_frame_has_transparency { + dispose = gif::DisposalMethod::Background; + } else { + // Workaround for Preview.app in macOS Big Oof + dispose = gif::DisposalMethod::Keep; + } vec![255; image.width() * image.height()] }; @@ -524,6 +531,12 @@ impl Writer { } } + // Check that palette is fine and has no duplicate transparent indices + debug_assert!(matches!(image8_pal.len(), 1..=256)); + debug_assert!(image8_pal.iter().enumerate().all(|(idx, color)| { + Some(idx as u8) == transparent_index || color.a > 128 || !image8.pixels().any(|px| px == idx as u8) + })); + let (left, top, image8) = if !first_frame && next_frame.is_some() { match trim_image(image8, &image8_pal, transparent_index, screen_after_dispose.pixels()) { Some(trimmed) => trimmed, diff --git a/gifski-api/src/ordqueue.rs b/gifski-api/src/ordqueue.rs index 93b457e7..0cffee18 100644 --- a/gifski-api/src/ordqueue.rs +++ b/gifski-api/src/ordqueue.rs @@ -1,8 +1,8 @@ use crate::error::*; +use crossbeam_channel::{Receiver, Sender}; use std::cmp::Ordering; use std::collections::BinaryHeap; use std::iter::FusedIterator; -use crossbeam_channel::{Sender, Receiver}; pub struct OrdQueue { sender: Sender>,